Page MenuHome

Basic optimization of FFT_NOISE.h
Closed, ArchivedPublicPATCH

Description

05-15-2012 Basic optimization of:

/blender/intern/smoke/intern/FFT_NOISE.h

Changes Include:

Removal of some redundant calculations.

Where possible, divisions now use multiplicative inverse.

Pulled time consuming math operations outside of loop bodies.

Post increment/decrement to pre increment/decrement for loops.

Compiles, executes, produces identical output, and improved bake times in test environment (Fedora 16 x86_64 Virtualbox Image) by approx. 3x under normal load, and approx. 1.5 under heavy load.

Event Timeline

Hello,

I really love that you took the time and effort to write this patch. But the problem is, that it does not produce the same results. Maybe visually, but not binary. I did a binary diff of the produces noise files and they do not match even with the same random seed.

Maybe you can update the patch so that the noise files are binary identicall?

Thanks!

Thanks for the reply. I took a look at my original submission and found where I expect the problem to be located.

The precalcuated variable inv_xHalf I defined as a float.

...float inv_xHalf = 1.0f / (float)xHalf;

In the original FFT_NOISE.c the value being used in the calculation is truncated to integer precision before being passed to the FPU before multiplying against the length of the distance vector.

...diff[2] * diff[2]) / (xRes / 2);

However, in the patch, the inv_xHalf variable does not get truncated, giving it full single float precision during the multiplication.

...diff[2] * diff[2]) * inv_xHalf;

I'm not currently able to rebuild or test, but this was the first section that stood out as posing a problem. I tested dummy output of my indexing to ensure I was putting the right values into the right places and those parts should be solid. So provided this is the only problem, the resubmitted patch should produce identical binaries.

*crosses fingers*

Good morning,

Thanks for the update.

It's still different though ;-)
The interesting thing is, that only *some* values are different and they differ only in 1 bit. Like having hex value "A6" instead of "A5" (always higher).

Sorry I couldn't get that fixed. I was really hoping that was the spot.

I think I'll have to get into a position where I can build again before I can hunt down the exact problem. Otherwise I could see this turning into a series of tweak edits that may or may not solve anything. In my mind this seems to be a floating point precision issue, but there are a few places in the code that it could be happening. Those few spots seem to reassure me that this is in fact the cause, but I wouldn't want to submit a series of patches that don't actually produce the desired result. I'm not sure how long it will be before I can get back on this, but when I do, I'll do all the tweaking myself, and ensure the new patch produces identical binaries before I even submit it.

If it helps at all, I was actually looking into a method to take Blenders approach to fluid dynamics and performing the calculations on the GPU via OpenCL. So, this optimized patch could ultimately prove itself useless in the event I can get that implemented.

Eeeeh… this is really old, guess we could close as archived?

Bastien Montagne (mont29) changed the task status from Unknown Status to Unknown Status.May 1 2014, 8:20 AM

One week without answer, closing.