Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Pu frac calculation #1168

Merged
merged 4 commits into from
Feb 9, 2023
Merged

Fix Pu frac calculation #1168

merged 4 commits into from
Feb 9, 2023

Conversation

mgjarrett
Copy link
Contributor

Description

The previous puFrac calculation had a numerator that was adjusted for density changes (from block expansion) but a constant denominator that did not account for expansion.

The new formula calculates number of Pu atoms directly and does not account for density in either the numerator or denominator.


Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any bug fixes or new features.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

Copy link
Member

@opotowsky opotowsky left a comment

Choose a reason for hiding this comment

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

Checkout that test!

Thanks, and this looks good to me! Confirmed to work in practice, as well.

@opotowsky opotowsky merged commit 40519da into terrapower:main Feb 9, 2023
@mgjarrett mgjarrett deleted the fixPuFrac branch February 9, 2023 19:15
@john-science john-science added the bug Something is wrong: Highest Priority label Feb 9, 2023
albeanth pushed a commit to albeanth/armi that referenced this pull request Feb 9, 2023
drewj-usnctech added a commit to drewj-usnctech/armi that referenced this pull request Jun 1, 2023
Moved the location to avoid re-setting for every child on the block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants