Skip to content

Allow DistribcellFilter to work with apply_tally_results=True - #3667

Merged
pshriwise merged 8 commits into
openmc-dev:developfrom
paulromano:distribcell-fixes
Dec 4, 2025
Merged

pshriwise merged 8 commits into
openmc-dev:developfrom
paulromano:distribcell-fixes

Conversation

@paulromano

Copy link
Copy Markdown
Contributor

Description

Currently, if you run a model with a tally that has a DistribcellFilter and try to use apply_tally_results=True, it raises an exception if you try to look at the tally results because the number of bins on the filter is not populated. This PR fixes it by making the following updates:

  • The number of bins for DistribcellFilters is now populated in the Tally._read_results method.
  • When a model is exported to XML, each Tally is marked with a _model attribute that references the Model that holds it, and each Filter is marked with a _tally attribute that references one of the tallies that holds it. This makes it so that we can determine geometry paths to cell instances, which are used when calling get_pandas_dataframe on a tally that includes a DistribcellFilter

A few other miscellaneous fixes are included in this PR:

  • When using apply_tally_results=True, if you look at the shape attribute of a tally, it has zero for the middle dimension (nuclides) when no nuclides are specified. Now it will show 1, corresponding to the default of "total".
  • The SearchResult class was not showing up in the openmc namespace because it wasn't specifically imported from openmc.model.
  • Make use of gnds_name where possible

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • 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)

@paulromano
paulromano requested a review from pshriwise December 2, 2025 15:57

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @paulromano! The updates are much appreciated.

Just one curiosity about linking data to the tally and filter objects.

Comment thread openmc/model/model.py Outdated
@paulromano

Copy link
Copy Markdown
Contributor Author

@pshriwise PR has been updated with the change you requested along with a unit test to confirm that the fix for DistribcellFilter works as intended

Comment thread openmc/model/model.py Outdated

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few small thoughts and then I think this is good to go. Thanks @paulromano!

Comment thread openmc/tallies.py
Comment thread openmc/tallies.py Outdated
Comment thread tests/unit_tests/test_filter_distribcell.py
Comment thread openmc/tallies.py Outdated
@paulromano

Copy link
Copy Markdown
Contributor Author

@pshriwise thanks for the review. Your comments have been addressed ✔️

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great @paulromano! Thanks for the fixes with sprinkles and a cherry on top! 🍒

@pshriwise
pshriwise merged commit db8d462 into openmc-dev:develop Dec 4, 2025
15 checks passed
@paulromano
paulromano deleted the distribcell-fixes branch December 4, 2025 21:58
apingegno pushed a commit to apingegno/openmc that referenced this pull request May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants