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
215 stars 86 forks source link

Potential for incorrect material properties upon restart or db load #1440

Open keckler opened 9 months ago

keckler commented 9 months ago

Whenever a material property is modified by a material modification and that modifier doesn't alter the material in such a way that the change is recorded in the database, the property will likely be wrong when the reactor is loaded from the database. This includes instances of just playing around with the reactor in iPython or in restarts/snapshots.

A concrete example of this is the TD_frac material modifier in either UraniumOxide or B4C. Because TD_frac (and its associated instance variable theoreticalDensityFrac) is not written to the database directly, it is left at its default when the reactor is loaded back up from the database. So if you do a restart run with a case that has either of those materials in it, your material densities will be wrong if your TD_frac entries don't exactly equal the default values in the material class. For UraniumOxide that is 1.0 and for B4C that is 0.90.

There are other materials, including ThoriumOxide and some lesser-used materials that will have a similar issue.

This same type of error is possible for other material modifiers as well. For instance if your thermal properties like thermal conductivity or heat capacity are dependent on composition as specified in the material modifications. I don't see any examples of this in the public framework.

I believe this has the potential to be causing lots of analysis problems, depending on how one is using restart cases in their workflow.

john-science commented 9 months ago

Yes, I believe you correctly understand the current situation.

The Database treats materials as static things.

But people can inject whatever code they want into the materials, of course. Including adding a lot of state to a material object. But all that arbitrary state information will be lost when you translate your run to the database.

This shouldn't affect most ARMI runs, but it would affect restarted runs. Mostly, it could affect post-run analysis.

I think it is infeasible to try to store any arbitrary materials state in the database. But if there is one important variable, it should be stored as a parameter on the component, and not on the material.

keckler commented 9 months ago

To clarify, I do not believe this bug is impacting the number densities stored as a component parameter, at least in the way that we are currently using the material's density() method.

In the current usage, c.material.density() is just used to instantiate the number densities. From that point on (at least in neutronics-type calculations), if we want to know the density ARMI actually looks to the number density parameter, since that is updated with burnup. This means that the number densities going into our neutronics calculations are not adversely impacted by this.

Other use cases may use c.material.density() in broader ways, though, which could be dangerous during restarts/snapshots.