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

OpenMC MSBR model and data #165

Merged
merged 55 commits into from
Dec 6, 2022
Merged

Conversation

yardasol
Copy link
Contributor

@yardasol yardasol commented Sep 26, 2022

Summary of changes

This PR adds an MSBR model for use by OpenMC using the same parameters in the Serpent MSBR model.

This PR also adds a script to download and convert the ENDF/B VII.1 library to an ACE library for use by Serpent. This ACE library can also be converted to the HDF5 format used by OpenMC.

Finally, this PR makes some minor bug fixes to typos present in the process_j312.bash script

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Required for Merging

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • My change is a source code change
    • I have added/modified tests to cover my changes
    • I have explained why my change does not require new/modified tests
  • My change is a user-facing change
    • I have recorded my changes in the changelog for the upcoming release
  • My change is exclusively related to the repository (e.g. GitHub actions workflows, PR/issue templates, etc.)
    • I have verified or justified that my change will work as intended.
  • All new and existing tests passed.
    • CI tests pass
    • Local tests pass (including Serpent2 integration tests)

Associated Issues and PRs

  • Issue: none

Associated Developers

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

@yardasol yardasol force-pushed the openmc-msbr-model branch 3 times, most recently from 7afaf2f to d40074b Compare October 22, 2022 03:45
@yardasol
Copy link
Contributor Author

@ZoeRichter I'd love your input on this.

the original model's material compositions did not
match what is in the Robertson report. This commit
fixes that for the OpenMC version of the model
Copy link
Contributor

@ZoeRichter ZoeRichter left a comment

Choose a reason for hiding this comment

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

Looks good overall! Normally I'd complain about the "c1", "c2", "c3", etc. variable names not being descriptive enough, but I think it's fine in this case because you set the long form names within the variables themselves.

Figure 3.3, pg 16

Model features:
- Simplified Zone IA (graphite element is idential Zone IIA in the model, does not match original paper spec)
Copy link
Contributor

@ZoeRichter ZoeRichter Nov 16, 2022

Choose a reason for hiding this comment

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

small typo, "identical to Zone IIA"

Suggested change
- Simplified Zone IA (graphite element is idential Zone IIA in the model, does not match original paper spec)
- Simplified Zone IA (graphite element is identical Zone IIA in the model, does not match original paper spec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!


Model features:
- Simplified Zone IA (graphite element is idential Zone IIA in the model, does not match original paper spec)
- Simplified Zone IIA (graphite element does not match the original spec)
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking to make sure I understand this correctly: Zone IA is identical to Zone IIA in the model, and neither match the specifications of Zone IIA from the paper? Could you mention how you changed Zone IIA in your model, compared to the specs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking to make sure I understand this correctly: Zone IA is identical to Zone IIA in the model, and neither match the specifications of Zone IIA from the paper?

I've been working on chapter 4 of my thesis which is a descriptin of the MSBR reference design, as well as the CSG model, and I've come to learn that this statement is actually incorrect. What is called "zone IA" in the model actually matches more closely in dimension to zone IB. I made a PR to adjust this, but failed to update the README.

Could you mention how you changed Zone IIA in your model, compared to the specs?

I plan to overhaul the README with a summary of Chapter 4 of my thesis once that chapter is complete. Stay tuned for this.


Missing from model:
- Zone IB
- Top part of core (filled with salt?)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is in the top of the core in the model? Is it just salt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

msbr_full_xz_ref

This is the xz-cross section of the core. I'm not sure what to call that rectangular bit sitting on top of the "main" vessel, but it seems to be part of the core. This is what I was referring to.


% ------ Vacuum cell
cell 57 5 void -3
cell 57 5 void

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check I'm reading the changes right - it looks like you've removed the outer void that surrounds the outside of the reactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not quite right. I've removed the surface that bounds the void cell (Universe 5). Universe 5 is only used in lattice elements in the main lattice, so having that universe bounded by a surface that is equivalent to the lattice element boundary is redundant. That's why i removed the bounding surface for Universe 5, since the lattice does the bounding instead.

%set rfw 1 restart
set inventory fuel all

%set printm 1 0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

for the settings you've commented out: are these things you anticipate needing again later, or can you remove them entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These settings are present in the original serpent input (msbr.serpent)

"template_input_file_path": "./msbr_endfb71.serpent",
"npop": 50,
"active_cycles": 20,
"inactive_cycles": 20,
Copy link
Contributor

Choose a reason for hiding this comment

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

are these pop/active/inactive values just for testing? They seem very low.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. They are so we make sure the model actually will run in saltproc (it doesn't right now as I detailed in issue #171)

examples/msbr/openmc_msbr_model.py Show resolved Hide resolved
The script in this directory downloads and processes the JEFF
3.1.2 cross section library -- as well as spontaneous and delayed fission neutron and
The scripts in this directory downloads and processes cross sections data for
use in Serpent and OpenMC
Copy link
Contributor

Choose a reason for hiding this comment

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

"The scripts in this directory download and process"

yardasol and others added 2 commits November 17, 2022 13:07
We need more testing before changing the base composition

This reverts commit 0694808.
@samgdotson
Copy link
Contributor

I think @ZoeRichter should handle merging this

Copy link
Contributor

@ZoeRichter ZoeRichter left a comment

Choose a reason for hiding this comment

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

I think It looks good to merge.

cell 315 0 fuel -3004 3017 -3011 % Lower plenum
cell 307 0 hast -3006 3005 3015 -3013 % Radial vessel
cell 308 0 hast -3006 3013 -3014 % Top vessel
cell 309 0 hast -3006 -3015 3016 % Bottom vessel
cell 310 0 outside 3006 3016 -3014
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are very helpful :)

@ZoeRichter
Copy link
Contributor

@samgdotson merging is still blocked because you requested changes - if these have been resolved, I think this PR is good to go, and I'll merge it.

@yardasol
Copy link
Contributor Author

yardasol commented Dec 5, 2022

@samgdotson I think I addressed all of your changes

Copy link
Contributor

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

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

There are still some unanswered comments. Mostly, I want to know where all of these numbers come from -- some manner of scrutinizing the data -- rather than having them handed down from some higher power.
This might take the form of a README file for each example. A curious third party should be able to cross-reference the data from somewhere.

Comment on lines +15 to +19
fuel.add_components({'Li7': 0.0787474673879085,
'Be9': 0.0225566879138321,
'F19': 0.454003012179284,
'Th232': 0.435579130482336,
'U233': 0.00911370203663893},
Copy link
Contributor

Choose a reason for hiding this comment

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

The associated data is in examples/msbr/mats/msbr_saltproc_prepr_comp.ini where the values are also hardcoded but have a different "key."

I still want to know why these numbers and where they came from. The file above does not have this information either.

Comment on lines +501 to +508
main.universes = [[v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, d, z, z, z, z, z, z, z, z, z, u, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, v, v, v, d, z, z, z, z, l, l, l, l, l, l, l, l, l, z, z, z, z, u, v, v, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, v, d, z, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, z, u, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v],
[v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v],
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the shape you're trying to make here?

Comment on lines 957 to 981
"Am243": 212952778.98135212,
"Cm246": 220179493.9541502,
"U239": 196182721.4907133,
"Np239": 198519429.0542043,
"Th233": 185840570.73897627,
"Cf251": 223166396.6983342,
"Am242": 215146730.16368726,
"Cm245": 214624022.18345505,
"Cf254": 230600814.3619568,
"Am241": 211216798.63643932,
"Th232": 197108389.42449385,
"Cm240": 219583368.40646642,
"Th231": 186918512.14598972,
"Bk246": 224446497.874413,
"Cm247": 218956599.9139631,
"U238": 206851381.70909396,
"Bk250": 225432928.78068554,
"U230": 198841127.68309468,
"Cf249": 221434495.10716867,
"U234": 200632850.9958874,
"Cm250": 219425761.1783332,
"Th229": 192235847.44789958,
"Cm241": 219075406.6897822,
"Pu237": 210593272.23024797,
"Am240": 215272544.02927735,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just reiterating that I don't love having raw numbers like this. Ideally it would read something like

if deplete:
  fiss_q = read_values()
  ....
  # or 
  with open("data.pkl", "rb") as serp_data:
    fiss_q = serp_data.load()

It just seems unsustainable to me.

@yardasol yardasol requested a review from samgdotson December 5, 2022 21:40
Copy link
Contributor

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

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

It's nearly there!

Comment on lines +15 to +19
fuel.add_components({'Li7': 0.0787474673879085,
'Be9': 0.0225566879138321,
'F19': 0.454003012179284,
'Th232': 0.435579130482336,
'U233': 0.00911370203663893},
Copy link
Contributor

Choose a reason for hiding this comment

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

The readme for your thesis or for saltproc? I don't see any references to your (unpublished?) thesis in the saltproc readme. Perhaps it's in another PR?

Comment on lines +501 to +508
main.universes = [[v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, d, z, z, z, z, z, z, z, z, z, u, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, v, v, v, d, z, z, z, z, l, l, l, l, l, l, l, l, l, z, z, z, z, u, v, v, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, v, d, z, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, z, u, v, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v, v],
[v, v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v, v],
[v, v, v, v, v, v, d, z, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, l, z, u, v, v, v, v, v, v],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine, then. I wanted to see the shape you were going for and maybe check for some typos. It's another one of those things where a small error could generate considerable confusion later on.

Copy link
Contributor

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

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

I missed your comment about the json earlier.

@yardasol yardasol requested a review from samgdotson December 5, 2022 23:02
Copy link
Contributor

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

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

I'll approve this, although I hope future versions will have fewer hardcoded values. Data should ideally be read in from somewhere.

@samgdotson samgdotson merged commit 6bac510 into arfc:master Dec 6, 2022
github-actions bot pushed a commit that referenced this pull request Dec 6, 2022
github-actions bot pushed a commit to khurrumsaleem/saltproc that referenced this pull request Dec 7, 2022
github-actions bot pushed a commit to yardasol/saltproc that referenced this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants