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

Update b.p.heightBOL in Core::processLoading for thermal expansion at BOL #823

Merged
merged 17 commits into from
Aug 12, 2022

Conversation

albeanth
Copy link
Member

@albeanth albeanth commented Aug 5, 2022

Description

If inputHeightsConsideredHot: False, assemblies are thermally expanded from Tinput to Thot. This feature branch modifies Core::processLoading to update the BOL block height, b.p.heightBOL, to be the thermally expanded height rather than the cold height (as inputted in the blueprints).


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 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.

- needs assertion(s) and completion...
- needs testing + code cleanup/optimization
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
- was incorrect before to use `self.getAssemblies()`. To support both dbload: True/False, should pass in the specific list of assemblies that were expanded.
- also making it a protected method.
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
Summary:
1. updateBlockBOLHeights doesn't need core mesh params updated on dbload
2. manageCoreMesh should not be called for dbload.
    - setBlockMesh for assems will get set when assemblies are created from the blueprints in Core::createAsssemblyOfType
- manageCoreMesh should not be called during a dbLoad. rather, the blueprints assems are expanded, block BOL heights updated, and then have their block mesh set to the refAssem.getAxialMesh() on the existing r.core.
- The last part may not be needed, as setBlockMesh is called with createAssemblyOfType, but it's cheap to add and won't hurt anything.
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
@albeanth albeanth changed the title WIP: Update block bol heights Update b.p.heightBOL in Core::processLoading for thermal expansion at BOL Aug 9, 2022
@albeanth albeanth marked this pull request as ready for review August 9, 2022 19:57
@john-science
Copy link
Member

Forgive an ignorant question; I'm sure more people have talked about this change.

But isn't it confusing that we are changing the value of heighBOL? The BOL part of that name sounds like this value should be set at the beginning, and then never change.

At least, that's what the name sounds like to me.

@albeanth
Copy link
Member Author

albeanth commented Aug 9, 2022

@john-science that's a fair question! We changing it just once to ensure consistency between inputHeightsConsideredHot: True/False.

When True:

  • The block heights in the blueprints are supposed to be at hot temps. So when the core is built, b.p.heightBOL is stored at hot temps. This occurs during in the constructor of Block.

When False:

  • The block heights in the blueprints are supposed to be at cold temps. So when the core is built, b.p.heightBOL is initially stored using cold (i.e. room) temps (in the Block constructor). But in this case, during Core::processLoading the assemblies are thermally expanded to their hot temperatures. So to be consistent with the previous case, we need to update the b.p.heightBOL parameter in process loading after the assemblies have been expanded. This PR addresses this.

armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
armi/reactor/reactors.py Show resolved Hide resolved
armi/reactor/reactors.py Outdated Show resolved Hide resolved
Addresses comment here
- terrapower#823 (comment)

- Blueprints assems should be at hot height only. When created and placed into the core then they'll be set to the current core height (possibly thermal + irradiation expansion).
- Instead of doing twice (once at the onset and another if the block heights are updated), just do once at the end. This is simpler, cleaner, and easier to maintain.
- a ramification is that the runLog.extra statement in manageCoreMesh will not be printed during Core construction
- Addresses this comment terrapower#823 (comment)
- Also updating test_updateBlockBOLHeights to improve code coverage
- Fixing Tinput for fuel component in block_fuel3 in armi/tests test reactor.
and reformat unit test to catch bug. And is simpler than before.
- The bug: by passing in the finestAssemblyMesh.getAxialMesh() before the expansion happened, we were snapping the expanded meshes back to the non-expanded mesh and in turn, undoing the expansion... This isn't what we want. So this commit fixes that.
@albeanth albeanth merged commit e1e7e3d into terrapower:main Aug 12, 2022
@albeanth albeanth deleted the updateBlockBOLHeights branch August 12, 2022 14:42
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 2022
…n at BOL (terrapower#823)

* add update BOL heights when therm exp to Thot

* add test (WIP)

- needs assertion(s) and completion...

* expand blueprints assems at BOL

- needs testing + code cleanup/optimization

* correcting block BOL height update

- was incorrect before to use `self.getAssemblies()`. To support both dbload: True/False, should pass in the specific list of assemblies that were expanded.
- also making it a protected method.

* reorg axialExpChngr for dbload

Summary:
1. updateBlockBOLHeights doesn't need core mesh params updated on dbload
2. manageCoreMesh should not be called for dbload.
    - setBlockMesh for assems will get set when assemblies are created from the blueprints in Core::createAsssemblyOfType

* code clean up/reorg, fix dbLoad case

- manageCoreMesh should not be called during a dbLoad. rather, the blueprints assems are expanded, block BOL heights updated, and then have their block mesh set to the refAssem.getAxialMesh() on the existing r.core.
- The last part may not be needed, as setBlockMesh is called with createAssemblyOfType, but it's cheap to add and won't hurt anything.

* code cleanup + address comments

Comments addressed:
- https://github.com/terrapower/armi/pull/823/files#r939442958
- https://github.com/terrapower/armi/pull/823/files#r941630224
- https://github.com/terrapower/armi/pull/823/files#r939445205
- https://github.com/terrapower/armi/pull/823/files#r939443611

* fix order for updateBlocKBOLHeights with dbload

- if doing a dbLoad and detailedAxialExpansion: False, then setBlockMesh needs to be called after the BOL block heights are updated
- addresses comment  terrapower#823 (comment)

* creating test for Core::_updateBlockBOLHeights

* Update release notes.

* remove detAxExp from arg list, simplify logic

Addresses the following comments from review:
- https://github.com/terrapower/armi/pull/823/files#r942921304
- https://github.com/terrapower/armi/pull/823/files#r942926277

* initially set blueprints assems to hot height

Addresses comment here
- terrapower#823 (comment)

- Blueprints assems should be at hot height only. When created and placed into the core then they'll be set to the current core height (possibly thermal + irradiation expansion).

* simplify setting core axial mesh params

- Instead of doing twice (once at the onset and another if the block heights are updated), just do once at the end. This is simpler, cleaner, and easier to maintain.
- a ramification is that the runLog.extra statement in manageCoreMesh will not be printed during Core construction
- Addresses this comment terrapower#823 (comment)

* code cleanup and method rename for processLoading

- Also updating test_updateBlockBOLHeights to improve code coverage
- Fixing Tinput for fuel component in block_fuel3 in armi/tests test reactor.

* bug fix in _applyThermalExpansion for dbLoad

and reformat unit test to catch bug. And is simpler than before.
- The bug: by passing in the finestAssemblyMesh.getAxialMesh() before the expansion happened, we were snapping the expanded meshes back to the non-expanded mesh and in turn, undoing the expansion... This isn't what we want. So this commit fixes that.

* revert blueprints back to original state
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.

5 participants