Add New Component Axial Linking Approach #1376

albeanth · 2023-08-07T22:55:05Z

What is the change?

Closes #769. A request was made to improve the axial linking logic to take advantage of the block grids system in ARMI. This PR aims to accomplish that. While previous behavior is maintained, there are several changes:

AssemblyAxialLinkage::_getLinkedComponents now allows for multiple component axial linkage. Any components with multiple linkages are resolved in AxialExpansionChanger::retrieveLinkedComponent.
AxialExpansionChanger::retrieveLinkedComponent accounts for 3 cases to try and resolve the multiple axial linkage. See test_axialExpansionChanger.py::TestRetriveAxialLinkage for details.
Tests have been added and modified to account for new grid block behavior.
i. A block grid was added to armi/tests/detailedAxialExpansion/refSmallCoreGrid.yaml
ii. Additional complexity was added to the simple assembly defined in test_blockBlueprints.py.

Why is the change being made?

To support a user request for downstream analyses.

Checklist

This PR has only one purpose or idea.
Tests have been added/updated to verify that the new/changed code works.

The code style follows good practices.
The commit message(s) follow good practices. Keckler let me know that commit history isn't important, so the world gets the working branch!

The release notes (location doc/release/0.X.rst) are up-to-date with any important changes.
The documentation is still up-to-date in the doc folder. Another item for Formal Documentation of Axial Expansion in ARMI #935....
The dependencies are still up-to-date in setup.py.

Now need to do something with them....

- axial exp changer starting to account for block grids. - works when lattices have different numbers of pins. - need to change for case when pins are the same number in the grid (this should generate linking problems)

- still requires all pinned blocks to have the same number of pin groupings - but can now use the spatial locators of components to resolve multiple axial linkage

- uses a smarter grid-based approach. Score! - needs cleanup + unit testing

- also make it a class method to include self.a (instead of parent.parent....)

- during axial expansion choose the component which will be used for expansion. - all tests pass. Need to add aditional testing for coverage, improve docstrings, and see if logic optimization is available

- add testing to cover complexities

keckler

This looks very good. I like that your new test provides a good blueprint for somebody to implement this in their model.

I think the only thing that requires real attention is that I think the logic no longer tests if the components are solids. It'd be great if you could check on that.

keckler · 2023-08-15T13:39:55Z

armi/reactor/converters/axialExpansionChanger.py

+
+        Notes
+        -----
+        3 cases are considered, see test_axialExpansionChanger.py::TestRetriveAxialLinkage for details.


I think it could be more helpful to the user if this were reversed, i.e. put the description in this docstring and then reference it from the test docstring. People rarely look at docstrings for tests, so having the bulk of the description here instead seems to make more sense.

keckler · 2023-08-15T13:47:01Z

armi/reactor/converters/axialExpansionChanger.py

+        maxCompZtop = 0.0
+        for otherC in linkedComponents:
+            if otherC.ztop > maxCompZtop:
+                linked = otherC
+                maxCompZtop = otherC.ztop


Suggested change

maxCompZtop = 0.0

for otherC in linkedComponents:

if otherC.ztop > maxCompZtop:

linked = otherC

maxCompZtop = otherC.ztop

linked = max(linkedComponents, key=lambda c: c.ztop)

I think that accomplishes the same thing you're doing here.

keckler · 2023-08-15T13:54:54Z

armi/reactor/converters/axialExpansionChanger.py

-        self.linkedComponents[c] = lstLinkedC
-
-        if lstLinkedC[0] is None:
+                    if AssemblyAxialLinkage._determineLinked(c, otherC):


Genuinely curious -- is there a strong benefit to using AssemblyAxialLinkage._determineLinked() as compared to self._determineLinked() here?

keckler · 2023-08-15T14:03:39Z

armi/reactor/converters/axialExpansionChanger.py

-        component to compare and see if is linked to componentA
+        Notes
+        -----
+        If componentA and componentB are both solids and the same type, geometric overlap can be checked via


I think your change lost the check on whether the components are solids or not. Am I wrong about that?

keckler · 2023-08-15T14:09:16Z

armi/reactor/converters/axialExpansionChanger.py

+                # check for common indices between components. If either component has indices within its counterpart,
+                # then they are candidates to be linked and overlap should be checked.


Just thinking out loud here... It seems like this logic will not allow for partial-length fuel rods, if I'm reading it correctly. That's not an issue for me right now, but just trying to consider common cases (partial-length rods are common in BWRs).

keckler · 2023-08-15T14:27:47Z

armi/tests/detailedAxialExpansion/refSmallCoreGrid.yaml

@@ -19,3 +19,26 @@ core:
              IC  IC  MC  OC
            AF  IC  IC  PC  SH
  symmetry: third periodic
+
+fourPin:
+  # geom: hex


Suggested change

# geom: hex

keckler · 2023-10-09T15:33:44Z

I pushed some changes to this branch so that the axial expansion changer can properly handle blocks that don't have any solid components. This was leading to issues with assemblies accidentally changing overall height, which is not supposed to happen (the DUMMY block is supposed to compress to allow for the overall assembly height to remain unchanged).

~~I also added a test on this fix.~~

keckler · 2023-10-09T17:00:20Z

I do not know why this branch is now seeing some test failures in the bookkeeping module. I do not see any errors when I run those tests locally with 3.9, and I did not make any changes related to the bookkeeping module.

This reverts commit 73e7795.

This reverts commit 4f1dd9c.

This reverts commit bd9bfa6.

…components

keckler · 2023-10-10T19:45:38Z

Okay, after some offline discussion with @albeanth , the changes that I mentioned in the comment above have been rolled back. Instead of trying to handle them, we are explicitly going to block users from having blocks in their assemblies that don't have any solid components in them. In such scenarios, it is unclear how the axial expansion should be performed and so we just disallow it. The exceptions to this are:

The dummy block
An assembly with no solid components at all is allowed.

I added the appropriate logic and an associated test.

albeanth · 2023-10-10T20:59:53Z

armi/reactor/converters/axialExpansionChanger.py

+            if len(solidCompsInBlock) == 0:
+                raise InputError(
+                    f"Assembly {self.linked.a} is constructed improperly for use with the axial expansion changer.\n"
+                    "Consider using the assemFlagsToSkipAxialExpansion case setting."


Let's give a little more detail so users don't have to dig into the source code. E.g. "...is constructed improperly for use with the axial expansion changer; i.e., assemblies cannot have intermediate blocks void of a solid component. Consider using....

Well ideally there would be some documentation that outlines how to use the axial expansion capability ;-)

Hah! Yeah yeah.... very true....

…CompAxialLinkMthd

albeanth · 2024-02-25T17:03:00Z

@keckler a subtle hint hint you want this sooner than later, eh? 😄

keckler · 2024-02-25T17:10:49Z

I can't run my model without this branch, so at the very least I needed to get the merge conflicts resolved. But yes, sooner is better!

albeanth · 2024-02-25T17:20:22Z

I will carve some out time this week to restart this effort. I know John would like to see this PR gone too!

albeanth · 2024-03-13T16:14:18Z

Note to self: check in on BOL parameters on fuel. They're currently at the block-level and this change may break the validity of those parameters. They may need to get shifted to the components.

github-actions · 2024-09-07T03:29:07Z

This pull request has been automatically marked as stale because it has not had any activity in the last 100 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

albeanth · 2024-09-09T15:17:46Z

I'll just close this for now. This PR is tracked elsewhere so it's not going to get lost. When this work gets prioritized I'll reopen it.

john-science · 2024-10-03T18:56:59Z

@keckler @john-science We need to push this through.

albeanth · 2024-12-31T16:14:47Z

@keckler @john-science We need to push this through.

this will come through ~6 months.

albeanth added 18 commits August 4, 2023 11:01

WIP: have the pin groupings working.

8174fbe

Now need to do something with them....

adjusting blueprints to have for block grids

0a5b205

- axial exp changer starting to account for block grids. - works when lattices have different numbers of pins. - need to change for case when pins are the same number in the grid (this should generate linking problems)

resolve multiple axial linkage w/ spatialLocator

02c5685

- still requires all pinned blocks to have the same number of pin groupings - but can now use the spatial locators of components to resolve multiple axial linkage

add check to skip non-pinned assemblies

e789aa5

remove requirement for consistent pin groupings

cd3a1c7

- uses a smarter grid-based approach. Score! - needs cleanup + unit testing

rm unused methods

db8bbd0

rm unused instance variable

8098453

clean up a terrible docstring

1a1b347

retain original behavior for multiple linkage

115c3a9

- also make it a class method to include self.a (instead of parent.parent....)

improve docstrings

7f4ca22

make determineLinked and checkOverlap static methods

cf62f02

organizing and adding new tests

4eeef7a

adding test coverage for grid-based linking

ee9e252

rm duplicate test

169edc4

enable multiple axial linkage during set up

0d1a5b9

- during axial expansion choose the component which will be used for expansion. - all tests pass. Need to add aditional testing for coverage, improve docstrings, and see if logic optimization is available

add case3 to TestDetermineLinked

ab7cdb5

add additional complexities to test blueprints

1259c1f

- add testing to cover complexities

fix broken unit test

04a4e24

albeanth requested a review from keckler August 7, 2023 22:55

albeanth changed the title ~~Add new comp axial link mthd~~ Add New Component Axial Linking Approach Aug 7, 2023

keckler requested changes Aug 15, 2023

View reviewed changes

jakehader mentioned this pull request Sep 6, 2023

axialExpansionChanger and breakFuelComponentsIntoIndividuals() are not compatible #791

Closed

albeanth and others added 4 commits September 6, 2023 12:59

Merge branch 'main' into addNewCompAxialLinkMthd

9c795c8

Update ztop for blocks that don't have any solid components

bd9bfa6

Add test on assembly that has a purely fluid block

4f1dd9c

Remove use of walrus operator

73e7795

keckler added 2 commits October 10, 2023 16:35

Revert "Remove use of walrus operator"

63e37fd

This reverts commit 73e7795.

Revert "Add test on assembly that has a purely fluid block"

25f8e9f

This reverts commit 4f1dd9c.

keckler added 3 commits October 10, 2023 16:36

Revert "Update ztop for blocks that don't have any solid components"

989a43b

This reverts commit bd9bfa6.

Explicitly raise an error if assemblies include blocks without solid …

e3b4765

…components

Add test on new check

836be51

albeanth commented Oct 10, 2023

View reviewed changes

albeanth mentioned this pull request Oct 18, 2023

Misc cleanup #1439

Merged

8 tasks

Merge branch 'main' of https://github.com/terrapower/armi into addNew…

0c995d0

…CompAxialLinkMthd

Merge branch 'main' into addNewCompAxialLinkMthd

41127a2

albeanth mentioned this pull request Jul 5, 2024

Remove unnecessary spec for dummy block #1453

Closed

github-actions bot added the stale Inactive PRs cannot be open forever label Sep 7, 2024

albeanth closed this Sep 9, 2024

keckler mentioned this pull request Sep 30, 2024

Allow axial expansion related classes to be more extensible #1918

Closed

john-science reopened this Oct 3, 2024

john-science added enhancement New feature or request and removed stale Inactive PRs cannot be open forever labels Oct 3, 2024

terrapower deleted a comment from maurelius167 Oct 26, 2024

albeanth closed this Dec 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add New Component Axial Linking Approach #1376

Add New Component Axial Linking Approach #1376

albeanth commented Aug 7, 2023

keckler left a comment

keckler Aug 15, 2023

keckler Aug 15, 2023

keckler Aug 15, 2023

keckler Aug 15, 2023

keckler Aug 15, 2023

keckler Aug 15, 2023

keckler commented Oct 9, 2023 •

edited

Loading

keckler commented Oct 9, 2023

keckler commented Oct 10, 2023

albeanth Oct 10, 2023

keckler Oct 10, 2023

albeanth Oct 10, 2023

albeanth commented Feb 25, 2024

keckler commented Feb 25, 2024

albeanth commented Feb 25, 2024

albeanth commented Mar 13, 2024

github-actions bot commented Sep 7, 2024

albeanth commented Sep 9, 2024

john-science commented Oct 3, 2024

albeanth commented Dec 31, 2024

		# check for common indices between components. If either component has indices within its counterpart,
		# then they are candidates to be linked and overlap should be checked.

Add New Component Axial Linking Approach #1376

Add New Component Axial Linking Approach #1376

Conversation

albeanth commented Aug 7, 2023

What is the change?

Why is the change being made?

Checklist

keckler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keckler commented Oct 9, 2023 • edited Loading

keckler commented Oct 9, 2023

keckler commented Oct 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albeanth commented Feb 25, 2024

keckler commented Feb 25, 2024

albeanth commented Feb 25, 2024

albeanth commented Mar 13, 2024

github-actions bot commented Sep 7, 2024

albeanth commented Sep 9, 2024

john-science commented Oct 3, 2024

albeanth commented Dec 31, 2024

keckler commented Oct 9, 2023 •

edited

Loading