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

The description for `fpModelLibrary` is confusing #1701

Closed keckler closed 1 month ago

keckler commented 4 months ago

I have been tripped up by this description multiple times now. It is enough for me to assume that I'm not the only one who finds this confusing.

https://github.com/terrapower/armi/blob/1e356e7ee1756c4bde05eea1a9b9a8398e72c4f1/armi/physics/neutronics/fissionProductModel/fissionProductModelSettings.py#L57-L63

In particular, the part that confuses me is "Setting this is equivalent to adding all nuclides in the selected code library (i.e., MC2-3) within the blueprints nuclideFlags to be [xs:true, burn:false]."

This sentence would seem to imply that none of the isotopes are being burned, since all of them have burn: false.

My understanding is that this odd wording is in fact accurate, but it is strange because our internal depletion solver basically disregards the nuclide flags. For anybody without this additional context, though, this description probably makes no sense to them.

I would suggest one of the two options:

  1. Change the description to add the context needed so it makes sense
  2. Use the addOption method to pull this confusing option into our internal plugin so that external users are not confused by this
john-science commented 4 months ago

Man, more documentation doesn't make it good, eh?

How about something like this:

When `{CONF_FP_MODEL}` is set to `explicitFissionProducts`, this flag resets
all the nuclides in the selected library (e.g. MC2-3) in the blueprints
`nuclideFlags` to `[xs:true, burn:false]`. This is just a short-cut so that analysts
do not  need to change their inputs. (This flag, of course, will not guarantee
your downstream depletion code uses the nuclide flags.)
keckler commented 4 months ago

I think that your suggestion helps to make the description more concise, but it does not help to clarify my major confusion.

Specifically, I think the description needs to address the question of why it makes sense to set the nuclide flags for all isotopes to burn: false. According to this page in our docs, setting burn: false would lead to all isotopes not depleting. This is surely not what the user would want to happen.

@jakehader should be able to clarify this point.

wcscherer commented 2 months ago

A better description of the fpModelLibrary setting is being resolved in the TP internal depletion solver sqa docs. It is in review and I can post the description here so we can agree on a common wording.

wcscherer commented 2 months ago

@john-science Here is the reviewed and approved description from the TP internal depletion solver docs:

This setting should be used when fpModel: explicitFissionProducts. It is used in conjunction with any nuclideFlags defined in the blueprints to configure all the nuclides that are modeled within the core. Selecting any library option will add all nuclides from the selected library to the model so that analysts do not need to change their inputs when modifying the fission product treatment for calculations.