Add hook for UserWorkBeforeLoop #971
Conversation
Yurlungur
requested review from
jdolence,
pdmullen,
forrestglines,
brryan,
bprather,
pbrady and
pgrete
Thanks for the catches. Fixed. :) |
| @@ -94,6 +94,7 @@ class Mesh { | |||
|
|
|||
| // data | |||
| bool modified; | |||
| const bool is_restart; | |||
There was a problem hiding this comment.
I wanted people to be able to easily do different things upon restart vs first initialization, but without having to register two separate, maybe almost identical functions. There might be a better way than storing restart state in the mesh pointer, but it also seemed natural as something inherited from Athena++.
There was a problem hiding this comment.
What we've typically been doing is to check for tm.cycle != 0, which should also be possible in the new callbacks given that the SimTime object is passed.
There was a problem hiding this comment.
Good point---is_restart feels cleaner/less ambiguous to me, but I'm fine to remove this and tell people to use tm.cycle if preferred.
| void UserWorkBeforeLoop(Mesh *pmesh, ParameterInput *pin, SimTime &tm) const { | ||
| if (UserWorkBeforeLoopMesh != nullptr) return UserWorkBeforeLoopMesh(pmesh, pin, tm); | ||
| } |
There was a problem hiding this comment.
This copies the design of the other function hooks in state descriptor, but we could cut out the middle and just check for nullptr in the function pointer itself.
src/driver/driver.cpp
Outdated
| // Before loop do work | ||
| // App input version | ||
| if (app_input->UserWorkBeforeLoop != nullptr) { | ||
| app_input->UserWorkBeforeLoop(pmesh, pinput, tm); | ||
| } | ||
| // packages version | ||
| for (auto &[name, pkg] : pmesh->packages.AllPackages()) { | ||
| pkg->UserWorkBeforeLoop(pmesh, pinput, tm); | ||
| } |
There was a problem hiding this comment.
Many of the older function hooks store the function pointers in the mesh. I think this is a holdover from Athena++ and in parthenon, it's more natural to just query the app input or the state descriptor directly.
|
Does anyone from the AthenaPK team want to take a look at this before I merge it? Changes are pretty minor and just value-add, so maybe not necessary. @BenWibking @pgrete @forrestglines . |
| @@ -94,6 +94,7 @@ class Mesh { | |||
|
|
|||
| // data | |||
| bool modified; | |||
| const bool is_restart; | |||
There was a problem hiding this comment.
What we've typically been doing is to check for tm.cycle != 0, which should also be possible in the new callbacks given that the SimTime object is passed.
PR Summary
What it says on the can... This kind of workflow has come up a number of times for us in downstream applications, so I decided to finally stick it into parthenon. The basic need here is that you want to modify some variables after the mesh has been generated and problem generator has been called, for example, because you want to compute some global reduction variable, you want to normalize some field, you want to apply atmospheres, etc.
Now this can be done within a problem generator in some cases, for example with mesh-level problem generators. But it may be desired to do this work for every problem (for example). I add a hook in
ApplicationInputsand inStateDescriptor. As usual, you assign a function pointer, and then it gets called byEvolutionDriver.PR Checklist