Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More keyword changes: sobelN_a etc., #92

Merged
merged 20 commits into from
Dec 30, 2020
Merged

More keyword changes: sobelN_a etc., #92

merged 20 commits into from
Dec 30, 2020

Conversation

timhutton
Copy link
Member

@timhutton timhutton commented Dec 29, 2020

  • New stencils: sobelN_a, sobelNE_a etc. (all 8 for completeness)
  • Formulas now support a_e, a_nw etc. in a more natural way. Instead of accessing the float4 block that is the neighbor of the current block, these keywords return a float4 that has the four values that are neighbors of the cells. Now we can do e.g. a = a_ne; in a formula to have the whole pattern shift down and left by one pixel. This means there is much less swizzling to do to collect the cells you want in a float4 - we've done the swizzling for you.
  • Updated patterns to use the new features - see the files changed.
  • Added documentation in Help/writing_new_rules.html to explain it.

Copy link
Contributor

@danwills danwills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good from a brief look over diffs Tim, and am doing a build now... yep it compiled, launching.. yeah all good! Tried various formulas and it pretty much all seemed to be working.

I tried Cornus' "mutually-catalytic-spots-second-order-sobel.vti" and there seemed to be some 4-voxel-wide artifacts, especially as the initial state 'edge' disappears. I wondered whether there might be a slight issue there? (Maybe it was like that before?.. yeah it was!.. I tested in ready0.7 and it was the same, guess this is just part of the formula?)

I wondered what keywords we might need to add to make things like Cornus' 'smoke.vti' pattern into a formula? I think it's made out of cross-products and dot-products with neighbor-difference vectors, but maybe @CornuAmmonis could clarify (maybe it has a proper name too?) : ) It would be great to eventually have a full switchboard of different operators available for max-exploration-potential.

Now that the bilaplacian is built-in, can formulas like wave_equation.vti be converted to use a single reagent? or is the time-delay actually critical? (I am presuming because of how it is structured that the bilaplacian part is actually the bi-laplacian of the state from the frame-before-the-previous-frame, maybe I'm wrong about that?).

I think what I would like (though may be changing my mind) for rdyHoudini to be able to handle the new keyword stuff would be to split out the code that includes the various sections (by keyword) in AssembleKernelSourceFromFormula, into a separate method (so that it can be called independently).. However it's more and more seeming like maybe I should just get Ready to assemble a full kernel instead, and use that - which should be more compatible long-term as well. I'll investigate that avenue (and hopefully convert a few more things to formula-mode in the process? :))

None of this should hold up your pull request in the slightest! (Approved!) just some food for thought :)

@timhutton
Copy link
Member Author

timhutton commented Dec 29, 2020

@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
Copy link
Member Author

timhutton commented Dec 29, 2020

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
Copy link
Contributor

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
Copy link
Contributor

danwills commented Dec 30, 2020

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!

…I believe). Would love it if you could take a look Tim! Pull request owing if it's all-good.
@danwills
Copy link
Contributor

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
Copy link
Contributor

danwills commented Dec 30, 2020

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

a_out[index_here] = a + timestep * delta_a;

@danwills
Copy link
Contributor

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

…ges.html. Tweaked the initial pattern for faster startup. Tweaked the text.
@timhutton
Copy link
Member Author

timhutton commented Dec 30, 2020

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.

@timhutton timhutton linked an issue Dec 30, 2020 that may be closed by this pull request
@danwills
Copy link
Contributor

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.

@timhutton timhutton merged commit 462e99c into gh-pages Dec 30, 2020
@timhutton timhutton deleted the swizzle branch December 30, 2020 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kuramoto-Sivashinsky equation Pattern idea
3 participants