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

Formal Documentation of Axial Expansion in ARMI #935

Open albeanth opened 1 year ago

albeanth commented 1 year ago

Can we have some documentation added to the User Docs that details all that one needs to know related to thermal expansion in ARMI? I'm thinking an aggregation of all of:

To me, this would be incredibly valuable and would save many users lots of time.

I don't think such a thing should be part of the current PR, but reading through this PR left me craving exactly such a compilation because it is difficult (for me) to keep track of all of the many pieces.

Originally posted by @keckler in https://github.com/terrapower/armi/issues/931#issuecomment-1275240939

albeanth commented 1 year ago

I have started a feature branch that contains a standalone jupyter notebook that attempts to address some of these... But I think adding all of the other features that @keckler has requested here is likely outside the scope of a single jupyter notebook. And it's not immediately clear to me where we would put such documentation in the ARMI User Docs.

@john-science @ntouran @jakehader @onufer @opotowsky Do you have any preferences?

keckler commented 1 year ago

Perhaps here: https://terrapower.github.io/armi/user/inputs/settings.html#some-special-settings

albeanth commented 1 year ago

Perhaps here: https://terrapower.github.io/armi/user/inputs/settings.html#some-special-settings

Ooh, I like it.

opotowsky commented 1 year ago

It might also fit as a Tutorial? I know they are more interaction focused but for something move involved like this I could see an argument for that

albeanth commented 1 year ago

@opotowsky that was what I initially intended with the jupyter notebook. There might be an argument to have documentation in multiple places.

jakehader commented 1 year ago

I would suggest the gallery for examples, since that is where demonstration scripts should go. This would be in addition to some real documentation about how it works though.

john-science commented 1 year ago

I think a Jupyter notebook is a great idea. As long as it has (or comes with) actual test describing things. Working examples are great, when they come with lots of conversation. I would add an RST file along with the Notebook.

albeanth commented 1 year ago

Since GitHub doesn't feel like linking this....

https://github.com/terrapower/armi/discussions/1154#discussioncomment-4959351

albeanth commented 1 year ago

@john-science @mgjarrett @keckler

What do you think about adding a section in the ARMI docs (probably in the user-docs) that describes:

  1. Thermal expansion/contraction
  2. Radial expansion/contraction
  3. Axial expansion/contraction
  4. Disjoint axial mesh unification via uniform mesh converter
  5. Disjoint axial mesh unification via setBlockMesh

The first three are physics methods that should probably be documented.

The second two, though not physics, do have immediate impacts in how downstream simulations are set up and have direct and quantifiable impacts on the solutions.

Thoughts? Any other aspects we should include?

keckler commented 1 year ago

Yes. And some items that should be included within there are:

  1. What is the purpose of pseudo-density and/or some history on why we have/had density() and density3()
  2. Discussion of the inputHeightsConsideredHot setting and why that exists and when to use it
albeanth commented 1 year ago

why that exists and when to use it

This could probably be done for quite a number of settings.

albeanth commented 1 year ago

More things to add to the docs.... https://github.com/terrapower/armi/pull/1338

john-science commented 1 year ago

@albeanth Can you give us a quick run down of the state of this ticket, please?

Thanks!

albeanth commented 1 year ago

I have a local feature branch with some docs.... I just haven't made time to actually work on them. I'm hoping that in the fall I will have time to work on this some more. To totally close out this ticket will require time. In the meantime I just tag this issue with things I know we want to add to the docs. Sorry I don't have better news!

keckler commented 1 year ago

Another item on the TODO: https://github.com/terrapower/armi/pull/1342/files#diff-2e7e36a8449dfbb3e1763045f1dc6f274a93a332976b38e201b5c94cab552845R214

There is a similar docstring note in our internal implementation of the AxialExpansionChanger class as well.

john-science commented 6 months ago

@albeanth I would like to bump this work. It is awkward that we released this software to the NRC while this ticket was still open.

albeanth commented 6 months ago

I know. I agree... 😞

We have some documentation, but the potential scope of this documentation could be significant. We need to have some internal discussions to resolve this, I think.

albeanth commented 5 months ago

note to self in regard to choosing fuel component as the fuel block axial expansion target component:

If you override and make the target component the clad, yeah, you're gonna move fuel around between blocks (which would be a bad idea).