Why
post_advance (Simulator kwarg) is documented as "a python function to execute after each advance()" (simulator.py:84), but that's not what it does. Looking at advance():
if self._auto_dump() and self.post_advance is not None:
self.post_advance(self.cpp_sim.currentTime())
(simulator.py:181-182)
_auto_dump() is self.auto_dump and self.dump(), and dump() returns whether something was actually written to disk this step (activeDiagnostics.size() > 0, diagnostic_manager.hpp:249). So post_advance only fires on the subset of advances where a dump happened, and not at all if auto_dump=False.
Every real use of it in the codebase (test_2d_2_core.py, test_2d_10_core.py, td1dtagged.py) actually wants exactly that: they read back the HDF5 diagnostics that were just dumped and check something against them. So the current behavior is the right behavior for what it's used for, the problem is purely that the name and docstring describe something else, a per-advance hook, which it isn't and can't reliably be (it silently doesn't fire if auto_dump=False, or on steps with no dump due).
Someone reading "execute after each advance()" and writing a callback expecting it to run every step is going to be surprised it randomly doesn't, depending on their diagnostic timestamps and the auto_dump flag they may not even be touching on purpose.
What
Rename post_advance to something that names what it actually does, e.g. post_dump or post_dump_hook, so the coupling to dump events is explicit and expected rather than a surprise. Update the docstring to say it runs after a dump, not after every advance, and that it's a no-op on steps where nothing was due to be written.
Separately, worth deciding explicitly whether it should keep being gated by auto_dump too, right now if auto_dump=False the hook can never fire, even manually, since it's only ever invoked from inside _auto_dump() in advance(). If the intent is "run this whenever a dump happens", that should probably hold regardless of who's calling dump() and when.
see #1290
Ideas on how (optional)
No response
Alternatives considered
No response
Additional context
Came out of writing/reviewing the Simulator docstrings on pyphare-doc, related to the auto_dump scoping issue raised separately.
Why
post_advance (Simulator kwarg) is documented as "a python function to execute after each advance()" (simulator.py:84), but that's not what it does. Looking at advance():
if self._auto_dump() and self.post_advance is not None:
self.post_advance(self.cpp_sim.currentTime())
(simulator.py:181-182)
_auto_dump() is self.auto_dump and self.dump(), and dump() returns whether something was actually written to disk this step (activeDiagnostics.size() > 0, diagnostic_manager.hpp:249). So post_advance only fires on the subset of advances where a dump happened, and not at all if auto_dump=False.
Every real use of it in the codebase (test_2d_2_core.py, test_2d_10_core.py, td1dtagged.py) actually wants exactly that: they read back the HDF5 diagnostics that were just dumped and check something against them. So the current behavior is the right behavior for what it's used for, the problem is purely that the name and docstring describe something else, a per-advance hook, which it isn't and can't reliably be (it silently doesn't fire if auto_dump=False, or on steps with no dump due).
Someone reading "execute after each advance()" and writing a callback expecting it to run every step is going to be surprised it randomly doesn't, depending on their diagnostic timestamps and the auto_dump flag they may not even be touching on purpose.
What
Rename post_advance to something that names what it actually does, e.g. post_dump or post_dump_hook, so the coupling to dump events is explicit and expected rather than a surprise. Update the docstring to say it runs after a dump, not after every advance, and that it's a no-op on steps where nothing was due to be written.
Separately, worth deciding explicitly whether it should keep being gated by auto_dump too, right now if auto_dump=False the hook can never fire, even manually, since it's only ever invoked from inside _auto_dump() in advance(). If the intent is "run this whenever a dump happens", that should probably hold regardless of who's calling dump() and when.
see #1290
Ideas on how (optional)
No response
Alternatives considered
No response
Additional context
Came out of writing/reviewing the Simulator docstrings on pyphare-doc, related to the auto_dump scoping issue raised separately.