terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
232 stars 89 forks source link

axialExpansionChanger and breakFuelComponentsIntoIndividuals() are not compatible #791

Open Nebbychadnezzar opened 2 years ago

Nebbychadnezzar commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@albeanth @jakehader - Thoughts?

keckler commented 2 years ago

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

albeanth commented 2 years ago

@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 https://github.com/terrapower/armi/blob/c2d6c767ef0f5e95c76d9d3b21b03eab1d7d4636/armi/reactor/components/componentParameters.py#L118-L123. 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 commented 2 years ago

@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 commented 2 years ago

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

I would give a strong "yes".

jakehader commented 1 year ago

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

albeanth commented 1 year ago

@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 commented 1 year ago

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 commented 4 months ago

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

albeanth commented 4 months ago

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 commented 1 month ago

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

albeanth commented 1 month ago

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 commented 1 month ago

@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 https://github.com/terrapower/armi/issues/769 same as @keckler posited in https://github.com/terrapower/armi/issues/791#issuecomment-1194252277

This is effectively the same as https://github.com/terrapower/armi/issues/769 , but this is a much more concrete, likely, and understandable instance of the underlying issue

albeanth commented 1 month ago

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