Fixes two related bugs affecting depletion restarts - #4078
Open
GuySten wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes two related bugs affecting depletion restarts that chain continue_timesteps=True across multiple integrate(final_step=False) calls (e.g. running one depletion step at a time, updating model state between steps).
Bug 1: depletion_results.h5 has zero k/reaction rates for every step after the first
Integrator.integrate() writes a placeholder StepResult for the trailing "final simulation" when final_step=False (a zero source rate is passed to the operator, which resets but never runs transport, returning k=0 and zero rates). _get_bos_data() decided whether to reuse prev_res[-1] as beginning-of-step data based only on step_index == 0 — which is true on every restart's Integrator, regardless of whether prev_res[-1] is real data or that zero placeholder. The result: every continuation after the first silently inherited zero reaction rates for its Bateman solve and re-wrote the same zero placeholder back to disk, permanently.
Bug 2: restarted statepoints show a finite k-effective with infinite uncertainty
Separately, write_bos_data() (which triggers a statepoint write) was called unconditionally at the end of integrate(), even when final_step=False. Since no transport solve happened in that case (openmc.lib.reset() was called but openmc.lib.run() never was), the statepoint captured whatever partially-reset internal state was left in the C++ layer — observed as a plausible-looking nonzero keff paired with an infinite standard deviation.
Fix
Added an explicit StepResult.evaluated flag recording whether a step's k/rates came from a real transport/eigenvalue solve or is a final_step=False placeholder. Threaded through save(), distribute(), and the HDF5 read/write path (from_hdf5() defaults to True for pre-existing results files that predate this field, so old files keep working). VERSION_RESULTS bumped to (1, 4).
Integrator._get_bos_data() now checks prev_res[-1].evaluated (not just step_index) before reusing restart data, falling back to a real transport solve when the last saved step was a placeholder.
Integrator.integrate() now only calls write_bos_data() for the trailing step when final_step=True, so no statepoint is written when nothing was actually evaluated.
Testing
Added openmc/deplete/tests/unit_tests/test_deplete_continue_timesteps.py covering:
StepResult.evaluated round-trips through HDF5 save/load, including backward-compatible default for files without the field.
_get_bos_data() correctly distinguishes a genuinely-evaluated restart step from an unevaluated placeholder.
integrate() calls write_bos_data() once (not twice) when final_step=False, and twice when final_step=True.
Also manually verified against the pincell_depletion example using the reporter's exact restart-loop pattern: depletion_results.h5 now has correct, non-zero k/uncertainty at every completed step, and no spurious openmc_simulation_nN.h5 is written for steps that were never evaluated.
Fixes #3973
Checklist
I have run clang-format (version 18) on any C++ source files (if applicable)I have made corresponding changes to the documentation (if applicable)