Adaptive sampling improvements

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

Re: Adaptive sampling improvements

Post by B.Y.O.B. » Sun Apr 07, 2019 7:22 pm

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_combined.png
Old
old_samplecount_normalized.png
Old samplecount (normalized), brighter area = more samples
old_stats.png
Old stats
new_combined.png
New
new_samplecount_normalized.png
New samplecount (normalized)
new_stats.png
New stats
Support LuxCoreRender project with salts and bounties

User avatar
Dade
Developer
Developer
Posts: 3138
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade » 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 :?:

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: 2962
Joined: Mon Dec 04, 2017 10:08 pm
Location: Germany
Contact:

Re: Adaptive sampling improvements

Post by B.Y.O.B. » 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.
Support LuxCoreRender project with salts and bounties

User avatar
alpistinho
Developer
Developer
Posts: 156
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho » Wed Apr 10, 2019 4:33 pm

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: 2962
Joined: Mon Dec 04, 2017 10:08 pm
Location: Germany
Contact:

Re: Adaptive sampling improvements

Post by B.Y.O.B. » Wed Apr 10, 2019 5:03 pm

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.
Support LuxCoreRender project with salts and bounties

User avatar
Dade
Developer
Developer
Posts: 3138
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade » Wed Apr 10, 2019 6:27 pm

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: 156
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho » Wed Apr 10, 2019 6:31 pm

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: 3138
Joined: Mon Dec 04, 2017 8:36 pm
Location: Italy

Re: Adaptive sampling improvements

Post by Dade » Wed Apr 10, 2019 6:35 pm

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: 156
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 11, 2019 1:01 am

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: 156
Joined: Thu Jul 05, 2018 11:38 pm
Location: Rio de Janeiro

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 11, 2019 1:02 am

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