armi icon indicating copy to clipboard operation
armi copied to clipboard

Add New Component Axial Linking Approach

Open albeanth opened this issue 2 years ago • 10 comments

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:

  1. AssemblyAxialLinkage::_getLinkedComponents now allows for multiple component axial linkage. Any components with multiple linkages are resolved in AxialExpansionChanger::retrieveLinkedComponent.
  2. AxialExpansionChanger::retrieveLinkedComponent accounts for 3 cases to try and resolve the multiple axial linkage. See test_axialExpansionChanger.py::TestRetriveAxialLinkage for details.
  3. 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

  • [x] This PR has only one purpose or idea.
  • [x] Tests have been added/updated to verify that the new/changed code works.
  • [x] The code style follows good practices.
  • [x] 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 #935....
  • [x] The dependencies are still up-to-date in setup.py.

albeanth avatar Aug 07 '23 22:08 albeanth

~~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 avatar Oct 09 '23 15:10 keckler

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.

keckler avatar Oct 09 '23 17:10 keckler

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:

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

I added the appropriate logic and an associated test.

keckler avatar Oct 10 '23 19:10 keckler

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

albeanth avatar Feb 25 '24 17:02 albeanth

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!

keckler avatar Feb 25 '24 17:02 keckler

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

albeanth avatar Feb 25 '24 17:02 albeanth

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.

albeanth avatar Mar 13 '24 16:03 albeanth