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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

allowing different volume methods for diff mat #2691

Conversation

shimwell
Copy link
Member

@shimwell shimwell commented Sep 10, 2023

Description

A while back I tried PR #2653 but this changed the behaviour of the diff_burnable_mat too much for it's main use case.

diff_burnable_mat appears to be doing two things when set to True, it changes the materials volumes and makes extra materials. This PR attempts to separate out both of these things and allow the user to specify each one

This PR is an attempt to add another argument to CoupledOperator so that the manner in which the volume is found can be user specified. This maintains current behavior as default when diff_burnable_mat is set to True but also allows users to split up the materials so that their volumes are assigned to the cell volume instead of always being divided equally.

This PR attempts to add another argument called diff_volume_method which can be set to "divide equally" (default) or "match cell" (useful for fusion r2s and burn up)

Fig 6 in this fantastic paper 馃槃 suggests we get about 20% burn up at the front of DEMO reactor solid breeder blankets. Hence I'm keen to see the CoupledOperator working in this slightly different way for fusion simulations.

Fixes # (issue)

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)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

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.

If at first you don't succeed, try again 馃槃 I like the approach you have here. I just made a few small stylistic fixes and I think it's good to go. Thanks!

@paulromano paulromano enabled auto-merge (squash) September 25, 2023 20:31
@shimwell
Copy link
Member Author

Thanks Paul, style is always appreciated. Glad we found a method of getting the tweak to the diff_burnable_mat behaviour through, I shall be making use of this straight away

@paulromano paulromano merged commit 989875b into openmc-dev:develop Sep 26, 2023
18 checks passed
pshriwise pushed a commit to pshriwise/openmc that referenced this pull request Sep 27, 2023
Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
stchaker pushed a commit to stchaker/openmc that referenced this pull request Oct 25, 2023
Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
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.

None yet

2 participants