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

Set OpenMCOperator materials to the model materials when diff_burnable_mats = True #2877

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

Kladdy
Copy link
Contributor

@Kladdy Kladdy commented Feb 17, 2024

Description

In a post on the forum, a user mentioned that using diff_burnable_mats = True in depletion calculations causes an error message on the form of ERROR: Could not find material 6 specified on cell 1. The issue still persists on 0.14.0, and does not seem to have been solved by #2037.

This seems to be due to the materials.xml file not containing the differentiated materials which is caused by the fact that OpenMCOperator.materials is still set to the old list of materials. Hence the change outlined in the PR would update OpenMCOperator.materials in the case that diff_burnable_mats = True.

I am however not sure if this change should be done later in the OpenMCOperator.__init__ code than what this PR suggests, and frankly I am not sure if this is even the best way of solving this issue, but it did work for my own work.

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)

@Kladdy Kladdy requested a review from drewejohnson as a code owner February 17, 2024 19:54
@gridley
Copy link
Contributor

gridley commented Feb 18, 2024

Hey Kladdy! I had run into this issue before and didn't bother to fix it, and had instead resorted to a workaround. It kept me quite frustrated for a bit trying to figure out what was going wrong, so I'd love to help get this merged.

@gridley
Copy link
Contributor

gridley commented Feb 18, 2024

By the way, I'm guessing you were setting diff_burnable_mats=True as a kwarg to the depletion integrator? I found that this issue did not come up when differentiating the burnable mats before the integrator. Really, I think it would be best to deprecate the option of differentiating them as an argument inside the depletion integrator--it's just better separation of responsibilities that way.

@Kladdy
Copy link
Contributor Author

Kladdy commented Feb 18, 2024

@gridley I'm glad I'm not the only one with the same problem :-)

In my case, I am setting diff_burnable_mats=True in the call to openmc.deplete.CoupledOperator, not in the call to the integrator:

def run_depletion(model: openmc.model.Model):
  op = openmc.deplete.CoupledOperator(model, diff_burnable_mats=True, chain_file=...)
  cecm = openmc.deplete.CECMIntegrator(op, dt, power)
  cecm.integrate()

I then call this run_depletion function as

model = openmc.model.Model(geometry=geometry, settings=settings)
run_depletion(model)

So I am indeed differentiating the burnable materials before the integrator. I assume you mean that the differentiation is done before the model itself is constructed? Where did you do the differentiation?

@shimwell
Copy link
Member

shimwell commented Feb 29, 2024

@Kladdy I believe @gridley if refering to the ability to do a differentiate_depletable_mats on the model added by this PR #2718 prior to the operator and prior to the integrator

model.differentiate_depletable_mats()

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.

This looks like a reasonable solution. Thanks for the PR!

@paulromano paulromano merged commit b1b8a4c into openmc-dev:develop Jun 13, 2024
18 checks passed
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
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.

4 participants