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
222 stars 87 forks source link

Refactor expansion of elements to nuclides from blueprints #320

Open jakehader opened 3 years ago

jakehader commented 3 years ago

When loading a reactor there are functions/methods defined in the blueprints that result in elements being expanded into nuclides using the definition of their natural abundances. This process is sensitive to user settings within the nuclideFlags option in the blueprints as well as some user-defined "magic" settings defined in the autoSelectElementsToKeepFromSettings function:

https://github.com/terrapower/armi/blob/b709d3450f6b7a89376abed3a2bf9107bcfe39f2/armi/reactor/blueprints/isotopicOptions.py#L446

Two issues that I am running into with this are:

  1. The code is hidden and it's challenging from a user-perspective to understand why in certain instances elements are being expanded and why in other instances of elements are not (see: _resolveNuclides). Even with lots of digging and sort of knowing what to look for this took me ~10-15 minutes to find and understand.

  2. There is a different behavior with elemental expansion when loading from inputs (blueprints) versus loading from a database. One may argue that it is not the database's responsibility to perform the nuclide expansion and I would agree. Rather than this code being on the database, it should instead be in the client plugin code that is expecting nuclides to be expanded. I think that the same can be said about loading from the inputs as well (unless explicit expandTo options are set in the nuclideFlags section).

To fix this, I suggest the following:

  1. We introduce some form of plugin hook that can define requirements for expanding nuclides based on the uses.
  2. We don't permanently change the state of the reactor, core, etc. when we expand nuclides (except for when the expandTo options are explicitly set).
  3. We refactor the elemental expansion code to a place in the framework where client code can easily access it and make this elemental expansion modifications to any ArmiObject.

I think that this will help make this behavior much less surprising to a user/developer and will also allow us to move away from these legacy "settings-based" changes to the reactor state within the framework. In this way, client code can still make new settings to configure the expansion of elements, but this will be on a case by case basis and for certain defined situations.

As an example, the situation defined above becomes problematic when performing database loads from a case that used ENDF/B-VII.0 data (where carbon, vanadium, etc.) are not expanded and the data library is switched to say ENDF/B-VII.1 or ENDF/B-VIII.0 where the carbon, vanadium, and other nuclides must be expanded. I think that the elemental expansion methods may make sense to be included in the newer nuclides implementation that is being worked on.

What do we think, @youngmit and @ntouran?

youngmit commented 3 years ago

In general, I think we should be punting the decision to expand elements to isotopics closer to the specific plugins that carry such requirements. However, ARMI should make imposing those mappings really easy for the plugin developer to implement. One approach could be to add an optional filter argument to getNumberDensities() that ARMI can call on the number densities dictionary before returning it to the caller. This could either be an instance of a class that ARMI provides with desired elemental expansions defined, or a super-custom callable that does more fancy mappings. Then the plugin could do something like:

class FancyCodeInputWriter:
    nDensFilter = armi.reactor.ElementExpander(["C", "Be"])
    ...
    def writeInput(self, r):
        ...
        nDens  = block.getNumberDensities(filter=self.nDensFilter)

The upside here is that the plugin code is far less sensitive to the inputs that originally created a reactor object, and removes the need for the user to manually resolve the mutual elemental requirements of the codes that they wish to use. One wrinkle here is how we should handle situations where we wish to update the number densities within a plugin. In this case, the plugin is making a pretty big decision for everyone else about which isotopes/elements to represent in the reactor model.

ntouran commented 3 years ago

I like Mitch's suggestion a lot. From my understanding, the plugins that are writing information wouldn't have to choose which nuclides to represent, that'd be universal. They would have to choose how to remap the elements/isotopes combos that they have updated back onto the unchanging set of nuclides/elements.

Big risk here is what would happen if a plugin inconsistently used the filters. That's another thing to keep in mind for functions like:

I guess if these all failed if someone tried to get or set one of the ones that only the plugin knows about mapping that'd be fine.

john-science commented 3 weeks ago

@jakehader @ntouran Heya! @keckler were just trying to decide what you guys thought of the expandTo nuclide flag. From the outside, separated by time from when this was implemented, it seems to generate some non-physical ideas.

Is it something like "MC2 doesn't have cross sections for this nuclide, so we're going to exclude it up front"? If so, I feel like ARMI shouldn't be doing that excluding, but the downstream... "mc2 ARMI Plugin" should be doing that.

From the discussion here, it really looks like Mitch suggested exactly that.

jakehader commented 3 weeks ago

Hey @john-science - I think that the global state of nuclides and needing the set of nuclides initialized at the start of reactor data model construction makes it difficult to parse down the expansion of nuclides in a given plugin or area of interest. For example, the nuclides on the reactor data model, if not complete for depletion later in life then results in the parameter definitions expanding. Maybe this isn't an issue if we rethink this, but this could lead to an inconsistency between the input to codes and the reactor data model being a source of truth.

With that said, the nuclides flags and expansions could be handled by plugins or at an application level versus assumed as inputs from the data model and that might make it easier to configure.

The issue in #1817 doesn't seem good though.