add lazy option to wave-op-mpi #185
Conversation
|
The CI appears to be failing for now. Request review when it's ready for a look or @-mention if it needs attention. |
|
This is ready for review @inducer @thomasgibson |
| if comm.rank == 0: | ||
| logger.info("dt = %g", dt) | ||
|
|
||
| t = 0 | ||
| t_final = 3 | ||
| istep = 0 | ||
| while t < t_final: | ||
| fields = rk4_step(fields, t, dt, rhs) | ||
| fields = rk4_step(fields, t, dt, compiled_rhs) |
There was a problem hiding this comment.
I think we might need to still perform the thaw/freeze pattern as illustrated in meshmode's simple-dg example. Maybe @inducer has more to say on this?
| fields = rk4_step(fields, t, dt, compiled_rhs) | |
| fields = thaw(freeze(fields, actx), actx) | |
| fields = rk4_step(fields, t, dt, compiled_rhs) |
Actually, I think my suggestion might not be the optimal way. The right way is probably illustrated here: https://github.com/inducer/meshmode/blob/main/examples/simple-dg.py#L494-L496 --- meaning we just wrap the rhs evaluations in a thaw/freeze pattern.
There was a problem hiding this comment.
Hmm, but isn't the thaw/freeze pattern due to using two different actx (like simple-dg does)?
I'm also confused why the ESDG example just thaws/freezes on the same actx:
grudge/examples/euler/acoustic_pulse.py
Lines 209 to 210 in 49bcb38
There was a problem hiding this comment.
I was under the impression that this was necessary to keep pytato's compiler from doing some unnecessary/repeated code generation between timesteps (which is an incredibly frustrating issue itself). I recall this being an issue for mirgecom as well for the temperature solver. Perhaps I am the one misunderstanding then
If I don't do the thaw/freeze in between calls to the timestepper, I experienced repeated code-generation that kills the simulation runtime. That's the summary of my issue. And I would expect that this example likely also suffers from that. I haven't checked out this branch to run it (yet) but are you not experiencing this problem with the example as is?
There was a problem hiding this comment.
I see, you're right! Should be fixed in 3e710a8
There was a problem hiding this comment.
For the PytatoPyOpenCLArrayContext, if we don't do this the fields starts becoming expression as below:
# at istep = 1
fields = fields(0) + dt/6*(k1(0) + 2*k2(0) + 2*k3(0)+k4(0))
# at istep = 2
fields = fields(0) + dt/6*(k1(0) + 2*k2(0) + 2*k3(0)+k4(0)) + dt/6*(k1(dt) + 2*k2(dt) + 2*k3(dt) + k4(dt))
I.e. the expression keeps growing in size. This is a consequence of us not thunking the values onto the expression objects. I agree this is a sub-optimal user-interface (Very related: inducer/arraycontext#102).
| if lazy: | ||
| fields = thaw(freeze(fields, actx), actx) |
There was a problem hiding this comment.
This is odd, and it shouldn't be here. What happens if you remove it?
There was a problem hiding this comment.
This is directly related to this discussion: #185 (comment). Unresolved so we can continue the discussion there.
No description provided.