Support different timestep units for depletion

[paulromano]

This PR implements support for the following units for depletion timesteps:

• seconds: s (this was the default before)
• minutes: min
• hours: h
• days: d
• burnup: MWd/kg

The last one relies on operator for determining the mass of heavy metal present at the beginning of depletion. Any of these units can be specified through a new timestep_units argument to the integrator classes, e.g.

integrator = PredictorIntegrator(operator, timesteps, power, timestep_units='MWd/kg')

As proposed in #1410, you can also mix units for different timesteps by passing an iterable of tuples for the timesteps argument, e.g.

steps = [(0.05, 'MWd/kg'), (0.10, 'MWd/kg'), (10.0, 'd')]
integrator = PredictorIntegrator(operator, steps, power)

in which case the timestep_units argument is ignored.

Closes #1410.

[paulromano]

One other possible design option for mixed units that just came to mind: instead of passing a list of tuples to timesteps, we could support passing an iterable of units to timestep_units.

[drewejohnson]

In reference to your comment about allowing "timestep_units" to be an iterable, I think I prefer this approach. That way the user only has to make one iterable. The only downside is a complicated setting where might be following an actual cycle with down periods and frequently changing powers. But even then, you have some representation of power vs time, even if power changes a lot. At least in my [limited] experience

The testing is very clean and avoids having to run full transport which is great 🎉

My major request would be adding an example or note in the documentation about this capability. The minimal example section would be a good location IMO.

Overall I really like this approach. Even though it forbids my new animal-based unit system 🐝

[paulromano]

@drewejohnson

In reference to your comment about allowing "timestep_units" to be an iterable, I think I prefer this approach. That way the user only has to make one iterable.

I'm a little confused. You prefer the current approach, or changing it to allow timestep_units to be an iterable? If we change it, you would need two iterables for mixed unit cases (one for timesteps, one for timestep_units). The advantage of that approach is that it would be consistent (each of timesteps, power, and timestep_units are allowed to be iterable). The advantage of the current approach is that it makes very clear what each of the timestep values corresponds to since they are paired together in a tuple. I could go either way on this.

[drewejohnson]

Sorry, that was not very clear. I do prefer the current approach, for the reasons you've laid out.

Having the units closer to the values they represent should hopefully make this less error prone. I can imagine a very frustrating and hard to debug case where the units were incorrectly placed in the timestep_units iterable, causing time step and powers to be incorrectly applied.

[rockfool]

Just a very small idea: can we add GWd/t as the alternative unit of burnup, though its conversion from MWd/kg is super easy.

[paulromano]

They are the same, no?

MWd/kg * (1 GWd)/(1000 MWd) * (1000 kg)/(1 tonne) = GWd/t

My personal opinion is that we should not include GWd/t as an option because "ton" is not well defined (ask someone how much a ton is and you may get different answers). MWd/kg on the other hand is unambiguous. I opted to not include "year" as an option for the same reason; there is no single answer to the question "how long is a year?" (365 d? 365.25 d? 365.2425 d?)

[rockfool]

They are the same. I come up with it because GWd/t is commonly used in many reference papers or design reports. I agree with your idea on "year". As I know, "ton" in GWd/tHM or MWd/tHM is usually "metric ton". Anyway, this way can avoid such kind of "misunderstanding". So this is just my tiny question.
Great job. Thanks.

[paulromano]

@drewejohnson Ok, we're on the same page then. Thanks! I believe I've addressed all your comments; let me know if you have further requests.

[drewejohnson]

One minor request then I'm good to go

[paulromano]

@drewejohnson Sorry, just occurred to me that you were requesting some documentation updates as well. I'll add those to this PR.

[drewejohnson]

: