[Feature] Implement a warning when facilities have the same input and output commodity

[gonuke]

What is the motivation for your feature request? Please describe what you want to use it for.

When a facility has the same input and output commodity, it can cause the DRE to direct all of that commodity to itself and not allow any other flows of that commodity. This is particularly likely for a Storage facility, and a warning should be issued to alert users.

Describe the feature you'd like.

Add a warning for archetypes where this could be a concern. The simplest implementation is archetype-dependent.

Describe alternatives you've considered.

One alternative is to classify this as an error rather than a warning.

Another alternative would be to make a toolkit that could be easily included and issue a warning when any pair of identified commodities overlap.

How can this issue be closed?

Add a warning to a minimal set of archetypes, especially Storage.

[gonuke]

Thoughts on a toolkit solution? @abachma2 @dean-krueger @katyhuff

[dean-krueger]

You could inject a function into EnterNotify() (a function that's common to all facilities, should only get run once, and should be able to see the right information) that takes a list of input buffers and output buffers and then runs through them and checks that none of the inputs are the same as the outputs, then gives that warning if they are? Ideally there'd just be something running in the background that did this, since that depends on users to manually enter all the right buffers, but they should only ever need to do it once unless they're adding buffers themselves?

[abachma2]

I think Dean's idea makes a lot of sense. I'm not sure if I have a preference of the EnterNotify() or the toolkit, but for either of them, that would be changes to the Cyclus kernel, right? As opposed to the archetype-dependent/specific solution mentioned above?

[dean-krueger]

I think so, except you'd need to then implement it in all the archetypes uniquely (add the injected function from the toolkit to EnterNotify() and give it the appropriate buffers), so maybe not?

[gonuke]

If there was a rigid standard nomenclature for input and output commodities, then it would be possible to do this entirely on the cyclus kernel side. At the very least, archetype developers will need to give lists of which variables are input commodities and which are output commodities to some standard function that exists somewhere...

I think we can do it before EnterNotify because it's a warning that only needs to happen once per prototype and not once per agent.

[abachma2]

Side note: Are we consistent in the Cycamore library regarding input and output commodity tags (e.g., in_commods vs incommod). If not, we might want to adjust that.

[gonuke]

Side note: Are we consistent in the Cycamore library regarding input and output commodity tags (e.g., in_commods vs incommod). If not, we might want to adjust that.

Not sure if we are consistent, but I know that some archetypes allow a single commod and others allow more than one

[abachma2]

Side note: Are we consistent in the Cycamore library regarding input and output commodity tags (e.g., in_commods vs incommod). If not, we might want to adjust that.

Not sure if we are consistent, but I know that some archetypes allow a single commod and others allow more than one

Right. I get that difference but it annoys me some that we aren't more consistent with the formatting. I'll make an issue for it and we can keep this one related to the warnings.

[dean-krueger]

it annoys me some that we aren't more consistent with the formatting

This has also frustrated me in the past @abachma2. I would be very glad to go through and check/change things one of these days if you don't get to it first.

[gonuke]

As far as I can tell, we don't have any easy way to interact with an arbitrary archetype before EnterNotify (or maybe Build), so that may be where it needs to happenn

[dean-krueger]

I think I've got something clever... one moment.