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
232 stars 90 forks source link

PercentBuMin parameter has `ParamLocation.MAX` tag #1980

Open drewj-tp opened 2 weeks ago

drewj-tp commented 2 weeks ago

Feels weird?

https://github.com/terrapower/armi/blob/d0f2ef81401c635328e72467ae5ab9eea9000e12/armi/reactor/blockParameters.py#L154-L159

We don't have a min tag. But also a "location" as "max" feels odd? Other locations are more spatially-oriented

https://github.com/terrapower/armi/blob/d0f2ef81401c635328e72467ae5ab9eea9000e12/armi/reactor/parameters/parameterDefinitions.py#L92-L103

Two potential resolution paths:

  1. Add a ParamLocation.MIN attribute and use that.
  2. Determine a better way to indicate if a parameter reflects some min / max / averaged quantity
john-science commented 2 weeks ago

@drewj-tp I count 16 usages of ParamLocation.MAX in ARMI, and 19 usages downstream.

Some parameters that use this:

But also:

So, I think you are seeing MAX, and want to see MIN. But it is written so that people see AVERAGE and MAX as opposing ideas. I'm not sure I see that as wrong or bad.

Thoughts?

drewj-tp commented 2 weeks ago

So, I think you are seeing MAX, and want to see MIN

That makes sense to me.

But it is written so that people see AVERAGE and MAX as opposing ideas

This does not make sense to me, nor is it communicated in the docs. I guess how often do people use these min / max tags rather than grabbing the parameters in question. We have some selection routines so you could grab all MAX parameters and make a report of that. That's probably good for lots of reporting purposes. But then you'd also see percentBuMin in that report which is not a maximum quantity