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

Apply decusping to individual assemblies. #1282

Merged
merged 16 commits into from
Jun 5, 2023

Conversation

mgjarrett
Copy link
Contributor

@mgjarrett mgjarrett commented May 30, 2023

Description


Grouping assemblies by type does not always work because assemblies of a single type can vary significantly in height over their lifetime in a reactor.

The particular example that motivates this bug fix is axial expansion caused by irradiation-driven swelling.

Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any bug fixes or new features.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

@mgjarrett mgjarrett requested a review from keckler May 31, 2023 19:44
@mgjarrett
Copy link
Contributor Author

I believe the coverage decrease is only because I deleted covered lines. No new uncovered lines were introduced, and unit tests were enhanced to provide more coverage of the new feature.

Copy link
Member

@keckler keckler 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 a couple questions that'd I'd like to get clarified before I approve this.

But nominally I think it looks fine.

Is there a way that you can show me how you are convinced that this is solving the problem that you're trying to fix?

And finally, are there performance or other issues that this will now cause that were leading us to not just go this route in the first place?

armi/reactor/converters/uniformMesh.py Show resolved Hide resolved
armi/reactor/converters/uniformMesh.py Show resolved Hide resolved
@mgjarrett
Copy link
Contributor Author

Is there a way that you can show me how you are convinced that this is solving the problem that you're trying to fix?

I will send you some evidence.

And finally, are there performance or other issues that this will now cause that were leading us to not just go this route in the first place?

I don't think there's a significant performance hit, but it does require paring down from a much longer initial list of material boundaries. The meshList length here is now going to be determined by the total number of assemblies instead of unique assembly types. In most cases all but 1 or 2 of them will eventually get pop'ed.

while True:
for i in range(len(meshList) - 1):
difference = abs(meshList[i + 1] - meshList[i])
if difference < minimumMeshSize:
if meshList[i] in anchorPoints and meshList[i + 1] in anchorPoints:
errorMsg = (
"Attempting to remove two anchor points!\n"
"The uniform mesh minimum size for decusping is smaller than the "
"gap between anchor points, which cannot be removed:\n"
f"{meshList[i]}, {meshList[i+1]}, gap = {abs(meshList[i]-meshList[i+1])}"
)
runLog.error(errorMsg)
raise ValueError(errorMsg)
if meshList[i + 1] in anchorPoints:
removeIndex = i
else:
removeIndex = i + 1
if warn:
runLog.warning(
f"{meshList[i + 1]} is too close to {meshList[i]}! "
f"Difference = {difference} is less than mesh size "
f"tolerance of {minimumMeshSize}. The uniform mesh will "
f"remove {meshList[removeIndex]}."
)
break
else:
return sorted(meshList)
meshList.pop(removeIndex)

@mgjarrett
Copy link
Contributor Author

The coverage decrease is again from removing previously covered lines. All of the new lines added are covered:
image

Co-authored-by: Michael Jarrett <jarremic@umich.edu>
@mgjarrett mgjarrett marked this pull request as ready for review June 5, 2023 16:25
@mgjarrett mgjarrett merged commit 90ac882 into terrapower:main Jun 5, 2023
drewj-usnctech added a commit to drewj-usnctech/armi that referenced this pull request Jun 12, 2023
…ss-iter-components

* terrapower/main:
  Improving HexBlock getWettedPerimeter calculation (terrapower#1299)
  Sort components on representative block. (terrapower#1275)
  Unifying the Reactor sorting (terrapower#1280)
  Building helper tool for when Parameters need to be NumPy arrays (terrapower#1292)
  Apply decusping to individual assemblies. (terrapower#1282)
  Block collection burnup (terrapower#1265)
  Cleaning up TODOs (terrapower#1291)
  Docstring Improvements - linting (terrapower#1287)
  Adding the word Important, per user request (terrapower#1289)
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