Making the FuelHandlerInterface be more responsible for shuffling and rotation

[drewj-tp]

I've been poking around the fuel handler interface and how it enables shuffling and rotation. And I have two suggestions in mind.

Traceability on shuffle logic

The shuffle logic script is the python file loaded by the settings that exposes some class that performs the fuel handling. Namely it implements logic to move assemblies around, discharging and creating fresh assemblies as necessary. I gather it's intended to be totally bespoke for each group.

But that shuffle logic file is, as far as I can gather, not stored in the database. That means it's impossible to re-produce someone's analysis just from the database. We store the blueprints and settings files but nothing related to the shuffle file.
Some thoughts

1. Eliminate the need for a shuffle-logic file and require the FuelHandlerInterface, or subclasses thereof, to also own the shuffling behavior, or
2. Write the contents of the shuffle logic file to the database, and allow that to be reproduced from just the database.

2 is probably the easiest, but it also feels like a security problem waiting to happen. Now you've got a binary file (database) that is producing a python file that is executed (potentially) without any user intervention. This could probably be mitigated by storing some checksum but it still gives me pause.

1 means the default FuelHandlerInterface that is shipped with ARMI won't work because it won't know how to shuffle assemblies. But that's kind of already the default state given we require people to bring their own shuffle logic file. 1 also means groups can have better version control over their shuffle logic, but it may be slower to test out new strategies. But maybe that encourages groups to have code review on their shuffle logic if that gets elevated in importance: not a throwaway script but an interface-level object.

Rotation

The rotation algorithm so far must be a function that is importable directly from the rotation algorithms submodule

armi/armi/physics/fuelCycle/fuelHandlers.py

Lines 115 to 120 in 864106e

	if hasattr(rotAlgos, self.cs[CONF_ASSEMBLY_ROTATION_ALG]):
	rotationMethod = getattr(rotAlgos, self.cs[CONF_ASSEMBLY_ROTATION_ALG])
	try:
	rotationMethod()
	except TypeError:
	rotationMethod(self)

(I don't like that try/except pattern but that's for a later time)

This means that groups attempting to develop their own rotation algorithm must make changes to the ARMI framework in order to have those picked up by the fuel handler. Additionally, the rotation is only fired if flux recon is active, which is 1) not documented as far as I am aware, and 2) not something that is facilitated by the open-source framework.

I propose we make the (functionally abstract) FuelHandler that is shipped in the framework more extensible for rotation. In that I mean we pull out that rotation code to a separate method that subclasses can override. Just like shuffling logic is tailored to a specific reactor and benefits from parameters defined within and without the public framework, one may expect rotation to require similar data.

The layout I have in mind looks similar to

class FuelHandler(abc.ABC):
    @abc.abstractmethod
    def chooseSwaps(self, factor: float):
        ...
    @abc.abstractmethod
    def performRotation(self):
        ...
    def outage(self, factor: float):
        # there's more logic presently that gets us to choseSwaps
        # but that's not written here for brevity
        self.chooseSwaps(factor)
        self.performRotation()

[john-science]

Traceability on shuffle logic

I have been shouted down on this several times, but several people. While I tend to agree with you, many of our users fiercely defend the shuffle logic being contained in an external "shuffle.py" file.

Just a head's up.

[drewj-tp]

Would love to hear the rationale there.

Reading the tea leaves, we'll need more traceability over things that aren't plugins but modify the reactor state. Right now it's impossible to reproduce analysis that performed shuffling just from the database. Which I think is the whole point?

Simple solution would just be to write the entire contents of the shuffle logic file to the database like we do the blueprints and settings file. We'd probably want a way to extract that as well if someone did want to reproduce the analysis

[john-science]

To be clear, I agree with you.

[keckler]

Devil's advocate -- Who says that we need to be able to reproduce an analysis given only a database? Speaking broadly, I would not say that is what is expected of calculations of record. A calculation can have multiple files that work in tandem, and this is very common.

I also wouldn't single out the shuffle logic file here. At the risk of ruining my own life, I will point out that there are other files that can do stuff during an ARMI run. You can point to a custom material class, for instance.

[john-science]

To reproduce an ARMI run, or analysis, it seems to me like we need:

1. Comparable hardware to run on (dubious, but that's an old-school rule)
2. The commit that ran the run/analysis.
3. The output database.
4. ....Other input data files? I guess this is the question.

For (4), certainly to reproduce an entire simulation, you would need many input data files (lots of external physics codes need them). So, that is the logic behind why shuffle.py is not in a commit now. It is just "data".

HOWEVER, it is actually a Python file and could, obviously, really, really easily be in the commit (2) with the rest of the code.