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

Remove use of mgxs module in MicroXS.from_model #2572

Merged
merged 2 commits into from
Jun 25, 2023

Conversation

paulromano
Copy link
Contributor

Description

Currently, the MicroXS.from_model classmethod has a dilute_initial argument that implements behavior similar to what has now been removed in #2568. In order to remove this from MicroXS, the first step is to get rid of its current reliance on openmc.mgxs to calculate microscopic cross sections (having had a look at mgxs, there's not a clear path in my mind for utilizing the multiply_density option there). This PR simply replaces the use of mgxs with manual creation of tallies that set multiply_density = False. The dilute_initial argument is preserved in this PR but will be removed in a future PR.

Checklist

  • I have performed a self-review of my own code
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)

@paulromano paulromano requested a review from yardasol June 21, 2023 21:59
@paulromano paulromano removed the request for review from drewejohnson June 21, 2023 21:59
Copy link
Contributor

@yardasol yardasol left a comment

Choose a reason for hiding this comment

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

Hi @paulromano,

Do you think at some point in the future it would make sense to use the mgxs module if/when we add support for multiple energy groups in MicroXS?

Even if this is the case, I like that we no longer have a bunch of duplicate flux tallies! I'll approve once the tests past!

openmc/deplete/microxs.py Outdated Show resolved Hide resolved
openmc/deplete/microxs.py Show resolved Hide resolved
@paulromano
Copy link
Contributor Author

@yardasol Even when we add support for multiple energy groups, I don't think mgxs will really give us much. It still has the problem that there's no clear path for utilizing multiply_density=False. In fact with the current setup in this PR, extending to multiple groups will be very straightforward because we are already using an EnergyFilter.

Copy link
Contributor

@yardasol yardasol left a comment

Choose a reason for hiding this comment

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

Good work @paulromano

@yardasol yardasol merged commit 19e4c37 into openmc-dev:develop Jun 25, 2023
@paulromano paulromano deleted the microxs-no-mgxs branch June 26, 2023 12:29
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