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

axialExpansionChanger and breakFuelComponentsIntoIndividuals() are not compatible #791

Open
Nebbychadnezzar opened this issue Jul 21, 2022 · 16 comments · May be fixed by #1990
Open

axialExpansionChanger and breakFuelComponentsIntoIndividuals() are not compatible #791

Nebbychadnezzar opened this issue Jul 21, 2022 · 16 comments · May be fixed by #1990
Assignees
Labels
bug Something is wrong: Highest Priority

Comments

@Nebbychadnezzar
Copy link
Contributor

An internal plugin runs Block.breakFuelComponentsIntoIndividuals(), which tears apart the fuel inside of a block (with multiplicity > 1) into individual components and gives them multiplicity = 1. This appears to adversely impact downstream plugins as well as other parts of ARMI, which appear to not be prepared for this (i.e., multiplicity = 1).

An example of this breaking other code is in the axialExpansionChanger module. An internal plugin uses this module, and I observe a failure in AssemblyAxialLinkage._getLinkedComponents(). This failure occurs because _determineLinked() is returning multiple matches for a given pin by matching it with the many pins in the neighboring block(s).

@Nebbychadnezzar
Copy link
Contributor Author

Nebbychadnezzar commented Jul 21, 2022

In speaking with @albeanth regarding this, he requested some discussion on the topic. As noted in the block module, breaking the pins into pieces is ultimately required for operations like pin depletion, so it doesn't seem like we can move away from it (since each pin needs to be able to track its own number densities).

@mgjarrett @john-science @albeanth @jakehader

@john-science
Copy link
Member

Well, I don't have a fully-formed thought on this yet.

One thing I will note right now is Block.breakFuelComponentsIntoIndividuals() has been in ARMI for 3-5 years now, so it's certainly not new functionality.

@john-science
Copy link
Member

@albeanth @jakehader - Thoughts?

@keckler
Copy link
Member

keckler commented Jul 25, 2022

This is effectively the same as #769 , but this is a much more concrete, likely, and understandable instance of the underlying issue.

@albeanth
Copy link
Member

@keckler you're exactly right with reference to #769 . It's the same problem.

As @Nebbychadnezzar points out, the axial expansion changer definitely does not support this currently. When we were discussing, we had a few ideas on how we could make it work. For axial linkage to work in the axial expansion changer, we'd need to know which pins are linked to each other. One idea we had was to use the component parameter pinNum

pb.defParam(
"pinNum",
units="N/A",
description="Pin number of this component in some mesh. Starts at 1.",
default=None,
)
. We could then match the pin numbers between axially adjacent blocks to determine if they are linked.

To use this though, we need to adjust breakFuelComponentIntoIndividuals to include the plenum blocks and any other pinned blocks as well - though there may not be fuel, there is clad and its expansion in the fuel blocks below will push up on the plenum clad.

If we choose to expand each pin individually, the cost of doing the expansion will increate - by how much, I don't know, but I'm fairly confident it won't be negligible. We'll be going from a handful of components per block to 100-200 components per block. That's a big change across an entire core and is going to come with a cost.

I was hoping to start a dialogue to see how often this functionality gets used and under what circumstances. I wonder if it makes sense to require plugins that need this to manage this deconstruction on their own - and then store relevant information on the database as needed - much like how armi/physics/neutronics/globalFlux/globalFluxInterface.py modifies a copy of the reactor as needed and maps/stores info back to the original ARMI Core.

@Nebbychadnezzar
Copy link
Contributor Author

Nebbychadnezzar commented Jul 25, 2022

@albeanth @keckler . So, it sounds like there are at least two use cases at the moment.

  1. Pin-level depletion - For a detailed assembly, we need to be able to track unique number densities for each fuel pin (and probably poison pin, although that isn't allowed right now?) in a block. It sounds like, in the past, we decided to track these number densities by creating unique pin objects for the pins.
  2. Unique pins within an assembly - As in @keckler's case, you can have situations where pins within an assembly can be different enough to model that difference (e.g., enrichment) by creating unique pin types for them.

In the extreme case of item 2, you could model an assembly where each pin has a different enrichment, and so each pin would be unique and have a multiplicity of 1. Is that a general case that we'd like ARMI to be able to model? That might inform the kind of architecture decision we want to aim for.

In the common case, though, assemblies typically have one pin type and a multiplicity that matches their total number.

@keckler
Copy link
Member

keckler commented Jul 25, 2022

Is that a general case that we'd like ARMI to be able to model?

I would give a strong "yes".

@jakehader
Copy link
Member

@albeanth is this related to changes in #1376 as well?

@albeanth
Copy link
Member

albeanth commented Sep 7, 2023

@albeanth is this related to changes in #1376 as well?

That PR was not made with this in mind. Let me take it for a spin...

@albeanth
Copy link
Member

albeanth commented Sep 7, 2023

Ok, so #1376 will not resolve this issue. It will technically run through the linkage and give you something, but won't be quite right I don't think. It looks like it'd be moderate level of effort, but should be a PR separate from #1376. If we could leverage the grids in breakFuelComponentsIntoIndividuals, that might help.

The other (and arguably more complicated) issue we run into is determining the axial expansion target component on the fly. Up to this point, we've been able to get around this by manually setting it in the blueprints via axial expansion target component. But that won't work here. There may be other issues too, not sure yet.

@john-science
Copy link
Member

@albeanth I would love to talk to you about the status of this issue. Since you are definitely the person who has the best eyes on it right now.

@john-science john-science added the bug Something is wrong: Highest Priority label Jun 15, 2024
@albeanth
Copy link
Member

This has not been on my radar. I don't know where (or if) this falls on the priority list. Pinging @zachmprince since this deals with pin-level stuff and may be something he and I need to think about for the next release of ARMI.

@john-science
Copy link
Member

@drewj-tp thinks this is possibly a symptom of a large axial expansion "linked components" issue.

@albeanth
Copy link
Member

albeanth commented Oct 3, 2024

This Issue can actually probably be closed. The method in question, breakFuelComponentsIntoIndividuals, is going to get removed as part of internal on-going pin-level depletion work.

It is a legacy method that is getting overhauled.

I'm curious to know what @drewj-tp's thoughts are on in regard to an issue with component linking. That's a pretty well tested and vetted routine.

@drewj-tp
Copy link
Contributor

drewj-tp commented Oct 3, 2024

@drewj-tp thinks this is possibly a symptom of a large axial expansion "linked components" issue.

I'm curious to know what @drewj-tp's thoughts are on in regard to an issue with component linking. That's a pretty well tested and vetted routine.

My suspicion is this is related to #1376 and #769 same as @keckler posited in #791 (comment)

This is effectively the same as #769 , but this is a much more concrete, likely, and understandable instance of the underlying issue

@albeanth
Copy link
Member

albeanth commented Oct 3, 2024

Yes, it is definitely related. It's the exact issue.

But the offending method of this issue is getting replaced and will be null in a few weeks. Once it gets removed, I was going to close this Issue (sorry, I could have communicated that).

@albeanth albeanth linked a pull request Oct 30, 2024 that will close this issue
6 tasks
@john-science john-science linked a pull request Oct 30, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants