Replace the IFP parameter enum with independent flags - #4094
Merged
Conversation
Contributor
|
Looking forward to these enhancements! |
GuySten
marked this pull request as ready for review
August 31, 2026 20:41
paulromano
approved these changes
Sep 1, 2026
paulromano
left a comment
Contributor
There was a problem hiding this comment.
@GuySten thanks for the simplification! This looks good to me. @JoffreyDorville did you want to look it over before we merge?
Contributor
|
@paulromano Yes, I'll have a quick look right now! |
JoffreyDorville
requested changes
Sep 1, 2026
JoffreyDorville
left a comment
Contributor
There was a problem hiding this comment.
Great work on the cleanup @GuySten ! I only have a single comment on renaming one variable, but other than that, it looks good.
JoffreyDorville
approved these changes
Sep 1, 2026
JoffreyDorville
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the update. I just replaced wants_delayed as well to be consistent. Looks good to me!
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.
Replace the IFP parameter enum with independent flags
No behaviour change and no regolds, with one small bug fix noted below. This is
the cleanup half of #3728, split out so the delayed-activation work can be
reviewed separately.
Problem
IFP state is currently held in three places that have to agree with each other:
IFPParameteris a two-bit set written out as four named states. Because it isan enum rather than two bits, turning one on requires a state transition:
and reading one back requires a decoder, which is what
is_beta_effective_or_both()andis_generation_time_or_both()inifp.cppexist to do. Between them they account for 24 call sites.
ifp_onis then a fourth piece of state that duplicates "is either bit set".Changes
The enum and the two decoders are gone. The two bits are stored as two bits:
and
ifp_onbecomes derived rather than stored, so it cannot fall out of stepwith them:
Setting the flags in
Tally::set_scores()is now a|=per score rather than astate machine, and every
is_*_or_both()call site reads the corresponding flagdirectly.
All existing
if (settings::ifp_on)guards are preserved asif (settings::ifp_on()). Nothing is executed that was not executed before, sothere is no performance question to answer here.
Bug fix
free_memory_settings()did not resetifp_onorifp_parameter. Both arederived from the tallies present in the model rather than read from XML, so a
second model loaded in the same process inherited IFP state from the first.
That is not only wasteful. The validation in
Tally::set_scores()is guarded byif (!settings::ifp_on), so with the flag already set the whole block wasskipped for the second model. A fixed source run with IFP scores would not have
raised "Iterated Fission Probability can only be used in an eigenvalue
calculation", and an out-of-range
ifp_n_generationwould not have been caught.This affects anything running more than one model per process: depletion,
openmc.libscripting, and the test suite.Both flags are now cleared in
free_memory_settings().Testing
No results change, so the existing IFP regression and unit tests cover this
unchanged. The validation reordering preserves the previous "check once, on the
first IFP tally" behaviour: validation runs while
ifp_on()is still false, andthe flags are set afterwards.
Notes
Follow-ups, kept out of this PR deliberately:
ifp.cppapplies the same operation to a
vector<int>stream and avector<double>stream, and
eigenvalue.cppcalls the MPI helpers once per stream. Foldingthe flag and both banks into one small type would remove most of that
duplication. Still no behaviour change, so it can land on its own.
results and requires regolds.
Checklist
I have followed the style guidelines for Python source files (if applicable)I have made corresponding changes to the documentation (if applicable)I have added tests that prove my fix is effective or that my feature works (if applicable)