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

add inputHeightsConsideredHot setting #657

Merged
merged 12 commits into from
May 20, 2022

Conversation

albeanth
Copy link
Member

@albeanth albeanth commented May 4, 2022

Description

Enable thermal expansion of assemblies when cores are built from blueprints + settings within armi.reactor.reactors.py::loadFromCs. The thermal expansion of each assembly is determined by the DeltaT of each component (DeltaT = Thot - Tinput).

To support backward compatibility (where no thermal expansion occurs) the setting, inputHeightsConsideredHot, was added. The default value for this is True. That way, current ARMI inputs retain their functionality. By setting to False, the armi.reactor.converters.axialExpansionChanger is spun up to expand each assembly as it is built.

To add a unit test that leverages this new capability (and that has the required dummy block for detailedAxialExpansion and the axialExpansionChanger), a new test reactor was created. The details:

  1. is based off of armi.tests.armiRun.yaml but sets:
axialExpansion: true
detailedAxialExpansion: true
  1. defines the required dummy block (dummy : &block_dummy) for armi.reactor.converters.axialExpansionChanger.py
  2. adds said dummy block to top of each assembly
    • dummy block height = 10% of original assembly height (i.e., total assembly height w/out dummy block)

Checklist

  • The code is understandable and maintainable to people beyond the author.
  • There is no commented out code in this PR.
  • The commit message follows good practices.

If user exposed functionality was added/changed:

  • Documentation added/updated in the doc folder.

Summary:
- is based off of armi.tests.armiRun.yaml but sets
    axialExpansion: true
    detailedAxialExpansion: true
- defines the required dummy block (`dummy : &block_dummy`) for armi.reactor.converters.axialExpansionChanger.py
- adds said dummy block to top of each assembly (dummy block height = 10% of original assembly height, i.e., total height w/out dummy block)
@albeanth
Copy link
Member Author

albeanth commented May 4, 2022

@john-science @jakehader

@john-science
Copy link
Member

I see that you've added new test files. But shouldn't I also see a new test that uses these files?

@john-science john-science added the testing Related to tests label May 4, 2022
@john-science john-science self-requested a review May 4, 2022 22:18
@jakehader
Copy link
Member

I see that you've added new test files. But shouldn't I also see a new test that uses these files?

Agreed. @onufer talked about adding a setting to control the thermal expansion on load and setting it false by default. Can we add that setting in this PR and then add a test that shows that the expansion occurs with this setting is enabled with these test inputs and then doesn't when it is disabled?

Summary:
- armi.settings.fwSettings.globalSettings.py: adding a setting, `inputHeightsConsideredHot`, to signal if inputted block heights (in armi blueprint) are at hot temperatures or cold temperatures.
- armi.reactor.reactors:py: adding option to thermally expand assemblies based on boolean value for `inputHeightsConsideredHot`.
    - NOTE: the number densities here ARE NOT correct. The number densities are expanded using the old (1/(1+dLL)^2) approach AND the expansion in axiallyExpandAssembly. A subsequent commit will come to correct this.
- armi.reactor.converters.tests.test_axialExpansionChanger.py: Adding a unit test to verify that thermal expansion does indeed occur during process loading. Two assertions: 1) assert that total assembly height is preserved; 2) assert that block heights are different.
- armi.tests.detailedAxialExpansion.refSmallReactorBase.yaml: set input temperatures all to 25.0 C
@albeanth albeanth changed the title add armi.tests.detailedAxialExpansion add inputHeightsConsideredHot setting May 16, 2022
- enables this option to be stashed on the core class and, ideally, accessible on various ArmiObjects
Summary:
- Actually utilizes setting, `inputHeightsConsideredHot`. This enables thermal expansion of each assembly based on the delta T between Tinpt and Thot of each assembly (by using the axialExpansionChanger)
- It also accounts for correcting number densities based on the boolean value of `inputHeightsConsideredHot`.

Details:
component.py:
- To correct number densities, component.py::Component is modified. The default apprach is that number densities are initialized at the cold/Tinput temperature. Then, if `inputHeightsConsideredHot = True`, `applyHotHeightDensityReduction` should be called.

reactorBlueprint.py:
- If `inputHeightsConsideredHot = False`, the axialExpansionChanger is invoked in _loadAssemblies.
- This will thermally expand each assembly based on the delta T between Tinput and Thot.
- If delta T is different across assemblies, assembly heights may end up being different and something may need to be done based on `detailedAxialExpansion`.

blockBlueprint.py
- logic included to account for reducing the densities based on hot heights if `inputHeightsConsideredHot = True`. This logic recovers backwards compatibility.

test_*.py
- updating tests to account for the above changes.
- note, these tests all assume that heights are inputted at hot dimensions and their passing helps ensure (or at least tries to) backwards compatibility
- more tests will need to be included for when `inputHeightsConsideredHot = False`
Summary:
- based on discussion with @jakehader, skipping thermal expansion of control assemblies (for now). There are CR designs in the literature that can axially expand _downwards_... This behavior is NOT accounted for currently in the axialExpansionChanger and therefore needs to be skipped.
- bug fix that calls the axialExpansionChanger twice when cores are built. Once in reactorBlueprint.py::_loadAssemblies (kept) and once in reactors.py::processLoading (removed).
- updating test_axialExpansionChanger.py::TestInputHeightsConsideredHot for new CR handling.
@john-science
Copy link
Member

@albeanth Is this PR ready to be merged?

@albeanth
Copy link
Member Author

@albeanth Is this PR ready to be merged?

@john-science yes, unless @ntouran, @jakehader, or anyone else wants to review.

@john-science john-science merged commit 0c7ef20 into terrapower:master May 20, 2022
@albeanth albeanth deleted the TestReactorDetailedAxialExp branch May 20, 2022 18:26
@albeanth albeanth mentioned this pull request Aug 1, 2022
7 tasks
jakehader pushed a commit that referenced this pull request Aug 3, 2022
PR #657 introduced a bug into how number densities are initialized for a component.

The bug:

When `detailedAxialExpansion: True`, number densities were being initialized with input temperatures.
This is incorrect behavior as number densities should be initialized with hot temperatures to account for radial expansion of components. Otherwise, number densities are never adjusted for radial expansion and masses are incorrect.

This PR fixes this by initializing the number densities with hot temperatures. Accounting for axial expansion via inputHeightsConsideredHot is then accounted for in adjustNDensForHotHeight.

In addition to this fix, I've changed method names and improved docstrings in the interest clarity.
Also cleaned up the code within `adjustNDensForHotHeight` to be simpler and more efficient (instead of completely recalculating the number densities previously calculated in `setNDensFromMassFracsAtTempInC`, it now just scales them accordingly.
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 2022
Enable thermal expansion of assemblies when cores are built from blueprints + settings within armi.reactor.reactors.py::loadFromCs. The thermal expansion of each assembly is determined by the DeltaT of each component (DeltaT = Thot - Tinput).

To support backward compatibility (where no thermal expansion occurs) the setting, inputHeightsConsideredHot, was added. The default value for this is True. That way, current ARMI inputs retain their functionality. By setting to False, the armi.reactor.converters.axialExpansionChanger is spun up to expand each assembly as it is built.

To add a unit test that leverages this new capability (and that has the required dummy block for detailedAxialExpansion and the axialExpansionChanger), a new test reactor was created. The details:

    is based off of armi.tests.armiRun.yaml but sets:

axialExpansion: true
detailedAxialExpansion: true

    defines the required dummy block (dummy : &block_dummy) for armi.reactor.converters.axialExpansionChanger.py
    adds said dummy block to top of each assembly
        dummy block height = 10% of original assembly height (i.e., total assembly height w/out dummy block)
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 2022
PR terrapower#657 introduced a bug into how number densities are initialized for a component.

The bug:

When `detailedAxialExpansion: True`, number densities were being initialized with input temperatures.
This is incorrect behavior as number densities should be initialized with hot temperatures to account for radial expansion of components. Otherwise, number densities are never adjusted for radial expansion and masses are incorrect.

This PR fixes this by initializing the number densities with hot temperatures. Accounting for axial expansion via inputHeightsConsideredHot is then accounted for in adjustNDensForHotHeight.

In addition to this fix, I've changed method names and improved docstrings in the interest clarity.
Also cleaned up the code within `adjustNDensForHotHeight` to be simpler and more efficient (instead of completely recalculating the number densities previously calculated in `setNDensFromMassFracsAtTempInC`, it now just scales them accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants