Adaptive sampling improvements

Discussion related to the Engine functionality, implementations and API.
User avatar
epilectrolytics
Donor
Donor
Posts: 561
Joined: Thu Oct 04, 2018 6:06 am

Re: Adaptive sampling improvements

Post by epilectrolytics » Wed Apr 03, 2019 4:09 pm

Dade wrote:
Wed Apr 03, 2019 3:45 pm
epilectrolytics wrote:
Wed Apr 03, 2019 3:31 pm
how well does this work in combination with OIDN?
I'm pretty sure it is not going to be a problem.
Great, that sounds like another welcome speedup, thanks alpistinho and Dade for these improvements!
MBPro 15" 16GB i7-4850HQ GT750M, MacOS 10.13.6 & Win10Pro PC 16GB Ryzen 2700X, 2 x RTX 2070

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

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 04, 2019 12:55 am

Dade wrote:
Wed Apr 03, 2019 10:13 am
Thanks, I merged the pull request on "new_conv_test" branch so we can work there without hurry. Let's start:

1) There seem to be a problem on borders. You are always dividing by 81 (9 * 9) however the pixels near borders have less neighbors so the convergence values are not correct.

2) Can we manipulate the metric to avoid sqrt() ? I mean, instead of comparing "sqrt(a) > sqrt(b)", I usually compare "sqrt(a * a) > sqrt(b * b) => a > b" so I can avoid the square root. sqrt() is something like 1000 times slower than a multiplication.

3) Do we still need to blur the convergence AOV ? You are already using a window of 9x9 around the pixel. I think the blur can be removed.

4) I have already changed some but, in general, use assert() instead for run time safety checks so they will not affect performance.
1) I believe it is fixed now. Should the window be configurable?

2) I don't think it going to be a performance issue. Rendering a 4096x2048 image, on my core i3-3220 these are the timings:

Code: Select all

[LuxCore][55.983] Convergence test step
[LuxCore][57.468] Convergence test: ToDo Pixels = 8360230, Max. Error = 1.64501 [421.123/256]
[LuxCore][88.228] Convergence test step
[LuxCore][89.421] Convergence test: ToDo Pixels = 5171077, Max. Error = 1.58259 [405.142/256]
So, about 0.5 s on a reasonably big image, and it don't even need to be run every pass.
Can I use OpenMP on this section to speed it up?

3) I've removed the code. Do you think it is worth trying a gaussian blur instead of the box I've used? The paper just averages equally like I did

4) I am testing for zeros on the denominator and I don't think there is any NaN and Inf problems anymore. I've removed the debug prints

I've tested with the luxball you've used on the original convergence post and I think it is working nicely there too (Just 10 Spp)
luxball_RGB_TONEMAPPED.jpg
Samplecount
Screenshot from 2019-04-03 21-42-20.png

About the development, I've pushed more commits to my branch, but since you've already merged, I don't know how it works now. Wouldn't it be better if the PR was kept open while this is not ready to be merged?

If someone has an idea for a pathological case where this breaks completely, I would be happy to know. Up until now it has been a fire and forget setting, but I am probably missing something.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 04, 2019 1:10 am

On the OpenCL side, where is the filmConvergence variable updated?

I can see the convergence test implemented on the sampler_sobol_funcs.cl file, but cannot find the convergence value being calculated.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by Dade » Thu Apr 04, 2019 7:54 am

alpistinho wrote:
Thu Apr 04, 2019 12:55 am
Can I use OpenMP on this section to speed it up?
Yes, it is used everywhere.
alpistinho wrote:
Thu Apr 04, 2019 12:55 am
3) I've removed the code. Do you think it is worth trying a gaussian blur instead of the box I've used? The paper just averages equally like I did
Box should be fine.
alpistinho wrote:
Thu Apr 04, 2019 12:55 am
I've tested with the luxball you've used on the original convergence post and I think it is working nicely there too (Just 10 Spp)
How do you have adaptive-ness at 10spp if the convergence test is run after 64spp ? The convergence AOV is updated for the first time after 128spp with default parameter and Sobol sampling will be uniform up to that point :?:
alpistinho wrote:
Thu Apr 04, 2019 12:55 am
About the development, I've pushed more commits to my branch, but since you've already merged, I don't know how it works now. Wouldn't it be better if the PR was kept open while this is not ready to be merged?
You should update your repository and work on new "new_conv_test" branch. You can just merge your current branch with "new_conv_test" branch and than fill a new PR.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by Dade » Thu Apr 04, 2019 8:00 am

alpistinho wrote:
Thu Apr 04, 2019 1:10 am
On the OpenCL side, where is the filmConvergence variable updated?

I can see the convergence test implemented on the sampler_sobol_funcs.cl file, but cannot find the convergence value being calculated.
The test is done on the CPU and the content of updated convergence AOV is than transferred to the GPU so there is no need to any new convergence AOV related code. You need only to update the Sobol OpenCL code. You will need also to modify the OpenCL host code to allocate the pass/pixel buffer as part of sampler shared data.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 04, 2019 8:59 am

Dade wrote:
Thu Apr 04, 2019 7:54 am

How do you have adaptive-ness at 10spp if the convergence test is run after 64spp ? The convergence AOV is updated for the first time after 128spp with default parameter and Sobol sampling will be uniform up to that point :?:
Isn't the warm-up and step parameters used for that? On this luxball test I just used 1 for both. I know a larger window would be recommended, but my computer is a bit too slow to render a 4k image.
Besides, after 128 app is much more difficult to see the difference in noise between the two algorithms.

On the other scene that I've used, I've used 10 samples as the step and the image was stopped at 100 Spp
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by Dade » Thu Apr 04, 2019 9:26 am

alpistinho wrote:
Thu Apr 04, 2019 8:59 am
Isn't the warm-up and step parameters used for that? On this luxball test I just used 1 for both.
Ok but it is not practical to run the convergence test every pass aside form tests, the overhead would be insane.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 04, 2019 10:59 am

Dade wrote:
Thu Apr 04, 2019 9:26 am
Ok but it is not practical to run the convergence test every pass aside form tests, the overhead would be insane.


Sure, but it is also nice to check the temporal stability of the method. Up until now it has been behaving nicely, even using a single pass as a step.

I will write the OpenMP version later today, and will set up the OpenCL drivers to compile this version. If there is no convergence calculation on the GPU side, it should be trivial to implement the new test there too.

The new test uses two comparisons and a subtraction, while the old uses just a comparison. Are GPU sensitive enough that two extra instructions like that are a problem?
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by Dade » Thu Apr 04, 2019 1:15 pm

alpistinho wrote:
Thu Apr 04, 2019 10:59 am
I will write the OpenMP version later today, and will set up the OpenCL drivers to compile this version. If there is no convergence calculation on the GPU side, it should be trivial to implement the new test there too.

The new test uses two comparisons and a subtraction, while the old uses just a comparison. Are GPU sensitive enough that two extra instructions like that are a problem?
Modifying Sobol OpenCL code is going to be simple. As usual, the ultra annoying part will be to allocate the shared buffer for keeping per pixel passes and pass around the pointer to that buffer.
Support LuxCoreRender project with salts and bounties

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

Re: Adaptive sampling improvements

Post by alpistinho » Thu Apr 04, 2019 1:17 pm

Since these are two orthogonal efforts, I will do the convergence test first and after this is working I will learn how to do the other part
Support LuxCoreRender project with salts and bounties

Post Reply