GollyGang / ready

A cross-platform implementation of various reaction-diffusion systems and PDEs.
GNU General Public License v3.0
761 stars 60 forks source link

More keyword changes: sobelN_a etc., #92

Closed timhutton closed 3 years ago

timhutton commented 3 years ago
timhutton commented 3 years ago

@danwills Good point about smoke.vti, it looks like it will be fine as a formula. I'll give it a go.

If there's a way for wave_equation.vti to be a single reagent that would be great but I don't think there is - the rate of change of a is an independent quantity and thus must be stored somewhere. Consider the case where a is all zero and b has an initial pattern.

(Of course you could interleave the a and b cells, or do similar tricks, but we don't want that.)

I guess we can get rid of the time delay in wave_equation.vti by updating b first:

b += laplacian_a * timestep;
delta_a = b;

And in fact this seems to be a better way of doing things - it now tolerates the damping being zero and the timestep can be larger. I'll update it!

timhutton commented 3 years ago

I've updated smoke.vti to be a formula and use the a_n etc. keywords. Let's look at the stencils it could use.

v combines input from a and b:

float4 v  = a_n - a_s - b_e + b_w + sq2 * (a_nw + b_nw) + sq2 * (a_ne - b_ne) + sq2 * (b_sw - a_sw) - sq2 * (b_se + a_se);

You could separate this into a and b inputs:

float4 v  = a_n - a_s + sq2 * (a_nw + a_ne - a_sw - a_se);
v  += - b_e + b_w + sq2 * (b_nw - b_ne + b_sw - b_se);

which would then be stencils like this:

 sq2     1    sq2             sq2     0   -sq2
  0      0     0   * a         1      0    -1   * b
-sq2    -1   -sq2             sq2     0   -sq2

but you'd have to separate out the sq2 values to avoid needing to hard-code them into the stencil.

So if we had stencils like this:

                      1  0  1
y_corner_gradient =   0  0  0
                     -1  0 -1

and

                      1  0 -1
x_corner_gradient =   0  0  0
                      1  0 -1

we could write something like:

float4 v = sq2 * y_corner_gradient_a + y_gradient_a + sq2 * x_corner_gradient_b - x_gradient_b;

I dont want to do this immediately but it certainly could be done for frequent use-cases.

danwills commented 3 years ago

I've updated smoke.vti to be a formula and use the a_n etc. keywords. Let's look at the stencils it could use. [...] I don't want to do this immediately but it certainly could be done for frequent use-cases.

It is super cool that you've been able to convert smoke.vti to formula mode Tim! Definitely indicates strong work with the keywords! The extended stencils could eventually be cool eventually too (maybe? if as you say they are also used elsewhere) but I think you are correct to resist adding just any-old-stencil that we find in the kernels, as we'd probably end up with a lot of custom single-use stuff in formula mode which I think/agree would not suit its design.

The new wave equation is totally great too! I wasn't expecting my ramblings to yield anything particularly useful, but you've totally made the whole thing one timestep more responsive (in a sense) top work! (This thing also went into (actual movie) VFX productions at work as our (float-valued) ripple-solver, so if the new version is really one-frame-more-responsive then we should totally update to your new version!)

I am also very much looking forward to having a play with the new keywords (once the dust settles slightly) as I've had enormous luck in the past with building a kind of "switchboard" formula (where you can blend in bits of all the various available operators) and I feel that this could lead to some interesting new experimental patterns. The fact that each neighbor is now a keyword gives huge possibilities, even on its own!

Top work @timhutton !!

danwills commented 3 years ago

For a small amount of context, smoke.vti went from having 94-odd lines down to 24 (and this is an example where some of the complexity is not part of the keywords (yet)), other cases shrink down by much larger orders of magnitude!

danwills commented 3 years ago

KSE Before: 78 lines. After: 3 lines!! this new keywords stuff is total genius:

delta_a = bilapweight * bilaplacian_a + lapweight * laplacian_a + ( gradient_mag_squared_a ) * gms_weight;
delta_a += a * stabilize;
a_out[index_here] = a + timestep * delta_a;
danwills commented 3 years ago

Ah whoops last line isn't even needed! down to two!

a_out[index_here] = a + timestep * delta_a;

danwills commented 3 years ago

Last line wasn't even really needed and I couldn't resist so KSE went from 78 lines to 1!

timhutton commented 3 years ago

Thanks Dan. I've promoted it out of experimental and made a few small tweaks - please check. Added it to CMakeLists.txt (so that it gets included in the build and the distribution package) and mentioned it in changes.html.

danwills commented 3 years ago

Thanks Dan. I've promoted it out of experimental and made a few small tweaks - please check. Added it to CMakeLists.txt (so that it gets included in the build and the distribution package) and mentioned it in changes.html.

I checked out your changes to the VTI and it's objectively a big improvement I reckon! Thanks heaps for tidying up my loose-ends!! :)

Still nothing blocking this pull-request in my opinion.