Skip to content

Fix manually set default importances not printed#921

Draft
MicahGale wants to merge 9 commits intodevelopfrom
892-default-importances-are-not-printed-in-newly-created-cells
Draft

Fix manually set default importances not printed#921
MicahGale wants to merge 9 commits intodevelopfrom
892-default-importances-are-not-printed-in-newly-created-cells

Conversation

@MicahGale
Copy link
Collaborator

@MicahGale MicahGale commented Mar 5, 2026

Pull Request Checklist for MontePy

Description

This fixes the bug where cells made from scratch almost never prints their importance in the output no matter if the importance were manually set (#892). Turns out that the parameter self._explicilty_set is set to True when the official setters are used, however, this value is never checked for formatting. To fix this was rather trivial.

However this issue brought up a question of behavior. In the test case cell.importance.all=1.0 is used on an unattached cell. It's assumed that this would set all importance. However, the documentation says that this only works on the particles in the problem. The code assumed that if the cell is not attached to a problem that cell.importance.all has no effect. This is rather intuitive. So I decided on the following behavior. Is this correct?

  1. With no problem importance.all sets the importance for every (~37) particle types.
  2. All particle importances are filtered first by problem.mode before printing. This is to avoid ~37 separate imp:X=1 clauses.

Finally, I noticed that in the tests for cell 4 only importance.neutron is set but the expected behavior is to print the default value for importance.photon. I decided to ignore this for now, and we can discuss.l

Fixes #892


General Checklist

  • I have performed a self-review of my own code.
  • The code follows the standards outlined in the development documentation.
  • I have formatted my code with black version 25 or 26.
  • I have added tests that prove my fix is effective or that my feature works (if applicable).

LLM Disclosure

  1. Are you?

    • A human user
    • A large language model (LLM), including ones acting on behalf of a human
  2. Were any large language models (LLM or "AI") used in to generate any of this code?

  • Yes
    • Model(s) used:
  • No

Documentation Checklist

  • I have documented all added classes and methods.
  • For infrastructure updates, I have updated the developer's guide.
  • For significant new features, I have added a section to the getting started guide.

First-Time Contributor Checklist

  • If this is your first contribution, add yourself to pyproject.toml if you wish to do so.

Additional Notes for Reviewers

Ensure that:

  • This PR fully addresses and resolves the referenced issue(s).
  • The submitted code is consistent with the merge checklist outlined here.
  • The PR covers all relevant aspects according to the development guidelines.
  • 100% coverage of the patch is achieved, or justification for a variance is given.

📚 Documentation preview 📚: https://montepy--921.org.readthedocs.build/en/921/

@MicahGale MicahGale linked an issue Mar 5, 2026 that may be closed by this pull request
@MicahGale MicahGale self-assigned this Mar 5, 2026
@MicahGale MicahGale added bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". critical An issue that seriously limits user adoption or hampers current use. bugs of unusual importance Bugs related to the Rube Goldberg machine that is the Importance class. labels Mar 5, 2026
@MicahGale MicahGale requested a review from tjlaboss March 8, 2026 04:55
@MicahGale MicahGale marked this pull request as ready for review March 8, 2026 04:55
Comment on lines +338 to +342
# Cell 4 should give "imp:n=4.0"
c4 = montepy.Cell(number=7)
c4.geometry = -problem.surfaces[1010]
c4.importance.neutron = 4.0
problem.cells.append(c4)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, I noticed that in the tests for cell 4 only importance.neutron is set but the expected behavior is to print the default value for importance.photon. I decided to ignore this for now, and we can discuss

Because the problem is in mode n p, then the photon importance needs to be printed as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So whenever any importance is set all importances for that cell should print?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All importances for the problem mode should print*, else MCNP will error out with X entries not equal to number of cells = Y.


*unless explicitly nullified for all cells, which is a feature that can be supported at a later date. See MCNP 6.3.1 Manual § 5.12.1:

Similarly, if an IMP: parameter appears on one cell card, a fatal error occurs if a
comparable entry does not appear on all cell card

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SMH.

for cell_num, imp_str in {
4: "IMP:n=2.0 IMP:p=2.0",
6: "IMP:n=1.0 IMP:p=1.0",
7: "IMP:n=4.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7: See above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugs of unusual importance Bugs related to the Rube Goldberg machine that is the Importance class. bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". critical An issue that seriously limits user adoption or hampers current use.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default importances are not printed in newly created cells

2 participants