-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adding temperature dependence when generating cross sections #1987
Conversation
Co-authored-by: Michael Jarrett <mjarrett@terrapower.com>
tempIntegratedVolume, volume = getBlockNuclideTemperatureAvgTerms(block, [nuclide]) | ||
return tempIntegratedVolume / volume | ||
|
||
|
||
def getBlockNuclideTemperatureAvgTerms(block, allNucNames): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this function didn't have any unit tests in ARMI yet? Damn.
@@ -503,6 +515,7 @@ def __init__( | |||
minDriverDensity=0.0, | |||
ductHeterogeneous=False, | |||
traceIsotopeThreshold=0.0, | |||
xsTempIsotope="U238", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only for homogeneous compositions or does it also work with other modeling options as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd guess it's independent of the treatment since this occurs in the XS group manager rather than specific modeling options applied for the input generation. Also, I assume that U238 is selected here because of fuel, but does this work if you select an isotope that isn't in the composition? For example if the isotope is not in the block, does this fail, should it fail, or should the environment/temperature group never be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for other based my expected implementation and on Mikes testing.
Johns recomendations Co-authored-by: John Stilley <1831479+john-science@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved.
This is a non-trivial re-write of xsgm.py
, so we very much should run the full suite of downstream testing before merging.
And this is an API-breaking change, so downstream PRs will be necessary.
def getBlocks(includeAll=True): | ||
return self.blockList | ||
|
||
core.getBlocks = getBlocks | ||
for b in core.getBlocks(): | ||
print(b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def getBlocks(includeAll=True): | |
return self.blockList | |
core.getBlocks = getBlocks | |
for b in core.getBlocks(): | |
print(b) |
Unless I'm missing something, it looks like this was just used while developing the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, if you dont do this, xsgm does all blocks in the core rather than just the block list, and you cannot do hand calcs to compare correct grouping. i would remove all blocks except those from the core, but i might have partial assemblies...
What is the change?
Add temperature dependent capability to armi when generating cross sections
Why is the change being made?
to be able to group similar temperature blocks for representative block construction
Checklist
doc
folder.pyproject.toml
.