Should the findAssembly method default to only finding assemblies with the FUEL flag?

[keckler]

The FuelHandler class has a powerful findAssembly method which is widely used to associate particular assemblies with a user's shuffling scheme. In the docstring for this method, it states:

typeSpec : Flags or list of Flags, optional
            only assemblies with this type list will be returned. If none, only fuel will be found.

If you look into the _getAssembliesInRings method which is actually used under the hood by findAssembly to pull up a list of all the assemblies, you'll see that the default screening for only fuel is actually applied in the signature there:

 def _getAssembliesInRings(
        self,
        ringList,
        typeSpec=Flags.FUEL,
        exactType=False,
        exclusions=None,
        circularRingFlag=False,
    ):

My question is, why should this method default to finding only fuel assemblies? It is not unheard of for other things to be shuffled (control assemblies, reflectors, etc.). Forcing the user to specify a flag using the typeSpec parameter of findAssembly when they want to shuffle anything other than fuel only makes findAssembly less powerful. I really don't see a good reason for this default, but I'm wondering if anybody else does.

[keckler]

I also see in findAssembly itself:

if typeSpec is None:
            # restrict motions to fuel only
            # not really necessary. take this default out if you want to move control rods, etc.
            typeSpec = Flags.FUEL

So I guess there is some redundant code in there forcing the default to look only for fuel assemblies.

But funny that the comment in the code acknowledges the question that I pose above.

[john-science]

This is strange. My first pass looking at it, and it seems like you're onto something, yeah. This method is such a monster, that I don't have a final answer for you yet. It is possible that such a monster method is being used in some surprising ways, so it isn't just used for FUEL. Though, that all leaves me feeling a bit uneasy.

[keckler]

Well we know that the method is not being used within the code itself, based on our discussion the other day. So it must just be used by users in their custom shuffle logic scripts. In that case, I would imagine that if we change the default, some people will need to update their custom shuffle logic to avoid accidentally getting things like control rods in their shuffles.

[jakehader]

Hi @keckler, as a part of our verification work for the fuel management there will be some proposed changes to improve this method. The biggest issue now is that this method is untested and it also has way too many arguments. I believe that this was written at a time when it was more difficult to just search for an assembly or set of assemblies that contain certain attributes or parameters. In today's ARMI framework, it should be pretty easy just to use a list comprehension or to pass in some "filter" function that can be user defined.

[john-science]

Passing a filter function seems doable.

Also, there must be a top list of 3-6 most common search types we could make into really generic, high-level functions that people would find useful and have fewer arguments.

As much as I wouldn't have written a method like this in the first place, I'm now worried that people have been using it for so many years, it would be a big change to try to get rid of it or change it significantly. At the very least, it will mean getting buy in from a lot of people and doing a version bump in ARMI.

[jakehader]

I don't believe that this method is current getting lots of us (or really anyone). Hunter (internally) is currently working on the requirements for the fuel management system and it might be good to link up with him if you get a chance. He is working with Natrium project stakeholders and Nick to gather needs for the API.