Adaptive sampling improvements

Discussion related to the LuxCore functionality, implementations and API.
User avatar
B.Y.O.B.
Developer
Developer
Posts: 4146
Joined: Mon Dec 04, 2017 10:08 pm
Location: Germany
Contact:

Re: Adaptive sampling improvements

Post by B.Y.O.B. »

I did a comparison with Sharlybg's food scene.
I used 200 samples halt condition and for the convergence test 10 warmup samples and 10 test step samples.

In my opinion, the noise is more evenly distributed with the new method. The wall is a bit more noisy, but the glass reflections and other more noisy areas are less noisy.
Attachments
Old
Old
Old samplecount (normalized), brighter area = more samples
Old samplecount (normalized), brighter area = more samples
Old stats
Old stats
New
New
New samplecount (normalized)
New samplecount (normalized)
New stats
New stats
User avatar
Dade
Developer
Developer
Posts: 5672
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade »

@Alpistinho: the new content of Convergence AOV ... it is not the convergence status anymore: it is more a "Relative error" AOV. The old Convergence AOV was converging to 0.0 as the rendering was going on, now instead it is pretty much constant because it is normalized between between min./max.

This break the compatibility with the past but it isn't a big deal. However the name doesn't make sense anymore and this is a bit more important.

We could introduce an additional AOV, for instance "Absolute error" AOV (similar to the old Convergence AOV, without normalization) and "Relative error" AOV (the new one used by samplers) to sort out somewhat the problem :?:

There is also the somewhat connected problem of expressing the halt threshold: before it was possible to use an intuitive values like 1/256 to have one shade level difference on an 8bit display; now it is very hard to pick a value.

We need to rework the test to establish if a pixel is done or not in order to be able to use a meaningful threshold value from the user point of view.
Support LuxCoreRender project with salts and bounties
User avatar
B.Y.O.B.
Developer
Developer
Posts: 4146
Joined: Mon Dec 04, 2017 10:08 pm
Location: Germany
Contact:

Re: Adaptive sampling improvements

Post by B.Y.O.B. »

I wonder how many people use the noise threshold as halt condition.

By the way, I was always a bit confused why the noise threshold is coupled with adaptive sampling.
It seems not intuitive to me (from a user perspective) - most of the time I want adaptive sampling, but no noise halt threshold, and sometimes it might be the other way around.
User avatar
alpistinho
Developer
Developer
Posts: 198
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho »

Hi,
Dade wrote: Wed Apr 10, 2019 3:58 pm @Alpistinho: the new content of Convergence AOV ... it is not the convergence status anymore: it is more a "Relative error" AOV. The old Convergence AOV was converging to 0.0 as the rendering was going on, now instead it is pretty much constant because it is normalized between between min./max.
This break the compatibility with the past but it isn't a big deal. However the name doesn't make sense anymore and this is a bit more important.
We could introduce an additional AOV, for instance "Absolute error" AOV (similar to the old Convergence AOV, without normalization) and "Relative error" AOV (the new one used by samplers) to sort out somewhat the problem :?:
That's true, this AOV could receive a rename. I can implement a new AOV that has the values used by the adaptive sampling and keep the old one, but I will need to compute both. I don't know if there are any performance considerations. I've mentioned this issue in the initial post
Dade wrote: Wed Apr 10, 2019 3:58 pm There is also the somewhat connected problem of expressing the halt threshold: before it was possible to use an intuitive values like 1/256 to have one shade level difference on an 8bit display; now it is very hard to pick a value.

We need to rework the test to establish if a pixel is done or not in order to be able to use a meaningful threshold value from the user point of view.
Yes, using a threshold still works, but the value is harder to guess now. I can revert it back for the todoPixels calculation
B.Y.O.B. wrote: Wed Apr 10, 2019 4:15 pm I wonder how many people use the noise threshold as halt condition.

By the way, I was always a bit confused why the noise threshold is coupled with adaptive sampling.
It seems not intuitive to me (from a user perspective) - most of the time I want adaptive sampling, but no noise halt threshold, and sometimes it might be the other way around.
I can also decouple the two if wanted and introduce some new adaptive.enable parameter. I was aiming at changing everything as little as possible, but since it is working now I can do some deeper changes
Support LuxCoreRender project with salts and bounties
User avatar
B.Y.O.B.
Developer
Developer
Posts: 4146
Joined: Mon Dec 04, 2017 10:08 pm
Location: Germany
Contact:

Re: Adaptive sampling improvements

Post by B.Y.O.B. »

alpistinho wrote: Wed Apr 10, 2019 4:33 pm I can also decouple the two if wanted and introduce some new adaptive.enable parameter.
I think adaptive sampling can be disabled by setting the "sampler.<samplerytpe>.adaptive.strength" property to 0.

It is also possible to disable the noise stop condition with "batch.haltthreshold.stoprendering.enable": https://wiki.luxcorerender.org/LuxCore_ ... conditions
But with the old adaptive sampling, we had to enable the noise halt threshold to be able to set the warmup and step properties which affect both adaptive sampling and the noise halt condition.

Maybe these properties could get a cleanup? "haltthreshold" as name doesn't really make sense in my opinion.
Something like this:
- To enable/disable adaptive sampling, keep the existing property: "sampler.<samplerytpe>.adaptive.strength"
- To enable/disable noise halt threshold, rename the "batch.haltthreshold" property to "batch.noisehaltthreshold"
- To control when convergence tests start and how often they are done, move some properties from "batch.haltthreshold" to a new group "convergencetest":
"convergencetest.warmup"
"convergencetest.step"
"convergencetest.filter.enable"
(These should be a separate group of properties because they affect both adaptive sampling and the noise halt threshold)

This would get rid of the weird "batch.haltthreshold.stoprendering.enable" workaround and make adaptive sampling and noise halt threshold independent, both using the "convergencetest.*" properties when at least one of them is used.
User avatar
Dade
Developer
Developer
Posts: 5672
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade »

B.Y.O.B. wrote: Wed Apr 10, 2019 4:15 pm By the way, I was always a bit confused why the noise threshold is coupled with adaptive sampling.
It seems not intuitive to me (from a user perspective) - most of the time I want adaptive sampling, but no noise halt threshold, and sometimes it might be the other way around.
As far as I remember, they are not more coupled with the new implementation. The sampling is now always proportional to the error, no matter of the halt threshold.

BTW, we could try to make it not linearly proportional but quadratic: to cut the noise in half you need 4 times the samples. Currently we are only doubling them, so using 4 more samples it should speed up the convergence.
Support LuxCoreRender project with salts and bounties
User avatar
alpistinho
Developer
Developer
Posts: 198
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho »

Dade wrote: Wed Apr 10, 2019 6:27 pm
As far as I remember, they are not more coupled with the new implementation. The sampling is now always proportional to the error, no matter of the halt threshold.

BTW, we could try to make it not linearly proportional but quadratic: to cut the noise in half you need 4 times the samples. Currently we are only doubling them, so using 4 more samples it should speed up the convergence.
You're right, but the threshold is still used to stop the rendering, just the value being different.

Another possible improvement is that I am normalizing the distribution after clamping the "convergence" inside 3 standard deviations from the mean, in order not to sample just a few pixels. The value of 3 is rather arbitrary and maybe could be exposed as a parameter. I can do that so people can play a bit with it
Support LuxCoreRender project with salts and bounties
User avatar
Dade
Developer
Developer
Posts: 5672
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade »

alpistinho wrote: Wed Apr 10, 2019 4:33 pm That's true, this AOV could receive a rename. I can implement a new AOV that has the values used by the adaptive sampling and keep the old one, but I will need to compute both. I don't know if there are any performance considerations. I've mentioned this issue in the initial post
I'm more worried of the memory usage that the performance, they should be both trivial to update. Basically, we could have:

#1 Convergence AOV with values between 0.0 and 1.0, optional stop the rendering when all values are under the halt threshold. Somewhat the old Convergence AOV.

#2 Noise AOV (Right name ? Anything better ?) with values between 0.0 and 1.0, used to drive adaptive sampling. The new Convergence AOV.

The sampler will require only the #2 so we are not going to increase the amount of memory required. #1 is useful for debugging, etc. (the halt test can be run even without storing the values in the AOV).
Support LuxCoreRender project with salts and bounties
User avatar
alpistinho
Developer
Developer
Posts: 198
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho »

I have wrote the changes to create the new NOISE AOV, and the necessary changes on the convergence test code.
I have broken LuxCoreUI and haven't been able to fix yet. I am getting:

Code: Select all

RUNTIME ERROR: Unknown FilmChannelType in Film::GetChannelCount(): 1073741824
I will try to fix it and do the suggested properties changes tomorrow.

As usual the changes are on https://github.com/Alpistinho/LuxCore, on the master branch. I will send the PR once it's done
Support LuxCoreRender project with salts and bounties
User avatar
alpistinho
Developer
Developer
Posts: 198
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho »

Dade wrote: Wed Apr 10, 2019 6:35 pm #1 Convergence AOV with values between 0.0 and 1.0, optional stop the rendering when all values are under the halt threshold. Somewhat the old Convergence AOV.
Do you want me to normalize this or should I use the same code as before?
Support LuxCoreRender project with salts and bounties
Post Reply