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.

Sign up for GitHub

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

Jump to bottom

Add get_mass() function to openmc.deplete.Results #2565

Merged
merged 5 commits into from
Jun 26, 2023
Merged

Add get_mass() function to openmc.deplete.Results #2565

merged 5 commits into from
Jun 26, 2023

Conversation

yardasol
Copy link
Contributor

@yardasol yardasol commented Jun 15, 2023 •
edited
Loading

Description

This PR adds a convenience function for getting mass or mass density of nuclides from depletion results.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format 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)

@yardasol yardasol force-pushed the mass-density-results branch from 0aa1bbd to 64725ee Compare June 15, 2023 09:03
@yardasol yardasol marked this pull request as ready for review June 15, 2023 09:04
@yardasol yardasol requested a review from drewejohnson as a code owner June 15, 2023 09:04
paulromano
paulromano requested changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Thanks @yardasol. A few suggestions below for you. Also, it looks like there's a merge conflict due to the merging of #2568.

openmc/deplete/results.py Outdated Show resolved Hide resolved
openmc/deplete/results.py Outdated Show resolved Hide resolved
openmc/deplete/results.py Outdated Show resolved Hide resolved
openmc/deplete/results.py Outdated Show resolved Hide resolved
openmc/deplete/results.py Outdated Show resolved Hide resolved
openmc/deplete/results.py Outdated Show resolved Hide resolved
paulromano
paulromano requested changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Sorry, missed a few nuc_units → mass_units before

openmc/deplete/results.py Outdated Show resolved Hide resolved
tests/unit_tests/test_deplete_resultslist.py Outdated Show resolved Hide resolved
tests/unit_tests/test_deplete_resultslist.py Outdated Show resolved Hide resolved
@paulromano
Copy link
Contributor

We're having some CI issues right now but once they're resolved, this is good to merge 👍

@shimwell
Copy link
Member

ah sorry I just noticed this has been approved already.

Please ignore my comments, if anyone needs other units we can always add them at a later date

@paulromano paulromano merged commit 51cf475 into openmc-dev:develop Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shimwell shimwell shimwell left review comments

@paulromano paulromano paulromano approved these changes

@drewejohnson drewejohnson Awaiting requested review from drewejohnson

@gridley gridley Awaiting requested review from gridley

Assignees
No one assigned
Labels
Depletion Python API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yardasol @paulromano @shimwell