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
226 stars 89 forks source link

Zones Improvements #563

Closed john-science closed 1 year ago

john-science commented 2 years ago

Zones has the potential to be a pretty powerful feature in ARMI, but has a handful of issues that make it impossible or unpleasant to use for certain cases. Below are a list of concerns/grievances that should drive the overhaul"

https://github.com/terrapower/armi/blob/22ebd1a2b6bdd0ebfd14b2e56273f8351db57f82/armi/reactor/zones.py#L401-L412

Sorry if it sounds like I'm being too down on Zones. I actually think they are a great idea; they are just really held back by their current design, and with a little polish could really fly.

jakehader commented 2 years ago

Hi @john-science,

I agree with this but internally we need to figure out where these functionalities should live if not on the zones objects in the framework. I've also mentioned this before, but should zones be a set of assembly locations or can they be a mix of any composite?

I'd also like to have a location in the database where zones can live and operator from. Maybe this is a separate table that has a reference to the "core" that the locations belong to, but this would go a long way in helping reuse "zones" across multiple uses if they are available or just allowing some analyst to ditch what is taken from the db and recreate them as needed. Maybe there could be a getZones on the database? Not sure.

john-science commented 2 years ago

@jakehader There are definitely several things we need to make decisions about before this ticket can be completed. Or even worked on. At the moment, this feels like an architecture/umbrella ticket to me. The sort of thing that might require some conversations to finish, or maybe even multiple, smaller tickets.

drewj-usnctech commented 2 years ago

Some disparate thoughts from our internal discussions on Zones

Could plugins define their own zone builders? This could clean up that large collections of conditions checking on zoning strategies. This might tie in nicely with your comment on

Zones should be created and owned by the interfaces that use them for whatever use they are build for. So sassys or rx coeffs would own what is currently core.zones.

A couple comments on serializing to/from data file. And that feels like something that would still provide value

jakehader commented 2 years ago

Thanks for the input @drewj-usnctech. We are having an internal meeting next week to kick off the zones updates and changing the implementation. We will make sure to update this ticket with requirements that we derive and check that they can also work for you before proceeding. Would you be interested in being a reviewer of any new implementation?

john-science commented 2 years ago

@drewj-usnctech Can you share with us (broadly) what you guys use Zones for?

We are in the process of redesigning / reorganizing them, for al the reasons shown above. And we don't want to drop your use-case.

drewj-usnctech commented 2 years ago

@jakehader I would be happy to be involved as much as you'd like. Glad our investigation into Zones coincided with your internal meetings

@john-science sure. We are currently investigating using Zones to define material regions that should be depleted together. In order to reduce the number of depletable regions in our Monte Carlo simulation while still capturing some spatial effects, we've decided to using the zoningStrategy to assign Zones to each assembly (well most of them - #800). The idea would be if you have the same fuel material in multiple rings of your reactor, each fuel material in each ring would be depleted with the same reaction rates, spectra, etc.

The extreme extension of this would be every Assembly being it's own Zone, and somehow axially dividing so that every Block is also its own Zone

jakehader commented 2 years ago

@jakehader I would be happy to be involved as much as you'd like. Glad our investigation into Zones coincided with your internal meetings

@john-science sure. We are currently investigating using Zones to define material regions that should be depleted together. In order to reduce the number of depletable regions in our Monte Carlo simulation while still capturing some spatial effects, we've decided to using the zoningStrategy to assign Zones to each assembly (well most of them - #800). The idea would be if you have the same fuel material in multiple rings of your reactor, each fuel material in each ring would be depleted with the same reaction rates, spectra, etc.

The extreme extension of this would be every Assembly being it's own Zone, and somehow axially dividing so that every Block is also its own Zone

@drewj-usnctech, Is it fair to say that in your use case you are describing zones also being applied to blocks as well as assemblies at the same time, such that both composites belong to some Zones object? Im trying to understand if we were to limit zoning on the core to a set of assemblies only if that would break your use case or if that's in alignment? I was throwing the idea around if Zones can just be a set of "any" composites and if it's required that they all be the same type or if we should allow the framework to allow this. Additionally, I'm curious from your perspective if having the zones be applied as a parameter in the database would be helpful. Do you ever have instances of multiple zones existing at a single time? If so, would a parameter that was something like a dictionary with {"zoneName": set(), ...} would make sense for the framework and parameter system to support

drewj-usnctech commented 2 years ago

@jakehader

Is it fair to say that in your use case you are describing zones also being applied to blocks as well as assemblies at the same time, such that both composites belong to some Zones object...I was throwing the idea around if Zones can just be a set of "any" composites and if it's required that they all be the same type or if we should allow the framework to allow this

I think this would be a welcome development. In doing this, we could also use zones to do axial depletion zoning. In a more extreme case, you would also maybe include individual Component objects in a zone to let each pin be it's own depletable region, but I think that's something that could be done by whatever interface is handling Block-based depletion.

Do you ever have instances of multiple zones existing at a single time?

I might be missing something here. Only having one zone seems like a case where everything is in the same zone. Maybe you're thinking different, potentially overlapping zones? Like a depletion Zones, a different Zones for reactivity coefficients, etc?

jakehader commented 2 years ago

@drewj-usnctech, sorry I meant to say that different zoning for different applications like rx coeffs, XS generation, thermal hydraulics, depletion, etc. not that there would only be one zone containing all the assemblies/blocks. My dictionary example wasn't communicated correctly.

john-science commented 2 years ago

@drewj-usnctech, sorry I meant to say that different zoning for different applications like rx coeffs, XS generation, thermal hydraulics, depletion, etc. not that there would only be one zone containing all the assemblies/blocks. My dictionary example wasn't communicated correctly.

Jake is just hoping we can simplify things a bit to just have one set of Zones in ARMI at a time. And we don't have to assume that you have 20 ARMI plugins during a run, each with their own set of Zones. Fingers crossed.

drewj-usnctech commented 2 years ago

@jakehader @john-science thanks for the explanations. Our current plan is just to have one set of zones for depletion and that's it. We'll be using xs types for xs generation rather than relying on zones

john-science commented 2 years ago

@drewj-usnctech I am working on the Zone redesign right now with @joshuavictorchen , and it looks to me (right now), that we could save a lot of time if we could make Zones only include Assemblies and Blocks. The reasons being, both of those have a getLocation() method in ARMI.

Do you really need Component-level Zones? I ask, because you mention it above, but you didn't sounds like you were really excited about that power. And supporting only Assemblies and Blocks LOOKS like it would significantly reduce the LOE we have on this task.

drewj-usnctech commented 2 years ago

@john-science I don't think Component-level Zones are necessary at this time. I imagine there might be a case for them somewhere but I'm struggling to think about it for now. Almost all of the use cases could be covered instead with some Block.getChildren(flags=...) type searching.

I feel good about only including Assembly and Block objects in Zones

john-science commented 2 years ago

Could plugins define their own zone builders?

@joshuavictorchen and I are currently redesigning Zones from the ground up. (Sadly, it seems they need such an overhaul.)

And in our redesign, I am currently working on exactly this: making zoning strategies a lot more flexible by exposing them at the Plugin level.

The current system just seems too inflexible, particularly for people like you, who may-or-may-not even be interested in pin-style reactors.

https://github.com/terrapower/armi/discussions/811

keckler commented 1 year ago

Is this closed with the merge of #943 ?