fix(importance): write importances set on newly created cells#923
Closed
novavale wants to merge 3 commits intoidaholab:developfrom
Closed
fix(importance): write importances set on newly created cells#923novavale wants to merge 3 commits intoidaholab:developfrom
novavale wants to merge 3 commits intoidaholab:developfrom
Conversation
added 3 commits
March 6, 2026 05:34
…ab#892) Three bugs prevented newly-created cells from having their importances serialized when writing the problem: 1. importance.all setter silently no-oped when the cell was not yet linked to a problem (_problem is None). The intent was lost before cells.append() was called. Fixed by storing the pending value in _pending_all_importance and applying it inside link_to_problem(). 2. importance.all setter raised KeyError for particles that weren't yet in _particle_importances (e.g. photon on a new neutron-only cell). Fixed by calling _generate_default_cell_tree() when the particle is missing, mirroring the existing __setitem__ behaviour. 3. has_information returned False for importances set to the default value (1.0) even when explicitly set by the user. Fixed by also returning True when _explicitly_set is True. Regression tests added for all scenarios from the issue report.
…licit importances Previously the test hardcoded start_idx=6 with a fragile check for the literal string 'imp:n=1', which broke when a programmatically-created cell emits 'IMP:n=1.0' (introduced by the importance fix in this PR). Replace with a robust FILL= token search: locate the word starting with 'FILL=', then skip 2 more range words (j, k), which always lands at the first universe number regardless of what precedes FILL in the line.
…daholab#892 - Apply black formatting to tests/test_importance.py - Add Bugs Fixed entry to 1.3.0 changelog for issue idaholab#892
Collaborator
|
This did not follow the PR template and the issue has already been assigned. |
Author
|
Thanks for the look. I can see #921 addresses the same issue — happy to close this one in favor of your implementation, or to fold any useful bits in if it helps. Let me know if there's feedback on the approach. |
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
2 participants
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.
Fixes #892
Problem
Three bugs prevented newly-created cells from having their importances serialized when writing the problem:
Bug 1:
importance.allno-ops beforecells.append()When setting
cell.importance.all = 2.0before the cell is added to a problem,self._problemisNoneand the setter silently did nothing. The intent was lost.Bug 2:
importance.allraisesKeyErrorfor missing particlesEven after linking to a problem,
all.setterwould crash withKeyErrorfor particles not yet in_particle_importances(e.g. photon on a freshly created neutron-only cell).Bug 3: Default-value importances not written
has_informationreturnedFalsewhen all importances equal the default (1.0), even if they were explicitly set by the user. This causedc3.importance.neutron = 1.0; c3.importance.photon = 1.0to be silently dropped.Fix
all.setter: When_problemisNone, store the value in_pending_all_importanceand apply it lazily inlink_to_problem(). Also call_generate_default_cell_tree()for any particle not yet present (mirrors existing__setitem__behaviour).link_to_problem()override: Flushes_pending_all_importanceacross all mode particles once the problem is known.has_information: ReturnsTruewhen_explicitly_setisTrue, ensuring default-value importances set programmatically are still written.Testing
4 regression tests added to
tests/test_importance.pycovering every scenario from the issue:importance.allset beforecells.append()test_892_importance_all_before_appendimportance.allset aftercells.append()test_892_importance_all_after_appendtest_892_importance_individual_default_valueimportance.all = 1.0(default) still writtentest_892_importance_all_default_value_still_writtenAll 952 pre-existing tests pass (2 unrelated pre-existing failures:
test_versionandtest_fill_multi_universe_order).📚 Documentation preview 📚: https://montepy--923.org.readthedocs.build/en/923/