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
226 stars 89 forks source link

`DerivedShape.getArea()` doesn't respect `cold=True` argument #1424

Closed keckler closed 1 month ago

keckler commented 1 year ago

Here is a very simple blueprints file that will run with just the framework:

nuclide flags:
  NA:
    burn: false
    xs: true
    expandTo:
  ZR:
    burn: false
    xs: true
    expandTo:
  C:
    burn: false
    xs: true
    expandTo:
  FE:
    burn: false
    xs: true
    expandTo:
  W:
    burn: false
    xs: true
    expandTo:
  SI:
    burn: false
    xs: true
    expandTo:
  CR:
    burn: false
    xs: true
    expandTo:
  MN:
    burn: false
    xs: true
    expandTo:
  NI:
    burn: false
    xs: true
    expandTo:
  V:
    burn: false
    xs: true
    expandTo:
  MO:
    burn: false
    xs: true
    expandTo:
blocks:
    fuel inner: &block_inner_fuel
        fuel:
            shape: Circle
            material: HT9
            Tinput: 25.0
            Thot: 450.0
            id: 0.0
            mult: 271
            od: 0.6029
        duct:
            shape: Hexagon
            material: HT9
            Tinput: 25.0
            Thot: 450.0
            ip: 14.922
            op: 15.710
            mult: 1.0
        intercoolant:
            shape: Hexagon
            material: Sodium
            Tinput: 25
            Thot: 450.0
            ip: duct.op
            op: 16.142
            mult: 1.0
        coolant:
            shape: DerivedShape
            material: Sodium
            Tinput: 25
            Thot: 450.0

assemblies:
    inner fuel:
        specifier: IC
        blocks:
            - *block_inner_fuel
        height: [1,]
        axial mesh points: [1,]
        xs types:
            - A
systems:
    core:
      grid name: core
      origin:
          x: 0.0
          y: 0.0
          z: 0.0
grids:
    core:
        geom: hex
        symmetry: third periodic
        lattice map: |
          IC

Run this and then load up the resulting DB to interrogate the component areas:

from armi.bookkeeping.db import databaseFactory
db = databaseFactory("anl-afci-177.h5", 'r')
with db:
     r = db.load(0,0)

for c in r.core[0][0]:
     print(c, c.getArea(cold=True))

<Circle: fuel> 77.3659281298001
<DerivedShape: coolant> 116.6107165005503
<Hexagon: duct> 20.904135052955258
<Hexagon: intercoolant> 11.916564981699693

for c in r.core[0][0]:
     print(c, c.getArea(cold=False))

<Circle: fuel> 78.13119460947178
<DerivedShape: coolant> 116.6107165005503
<Hexagon: duct> 21.1109086835348
<Hexagon: intercoolant> 9.802365546297814

Notice that the DerivedShape component has the same area no matter if cold=True or cold=False.

This means that the sum of the component areas changes, even though the op of the intercoolant region doesn't change.

sum([c.getArea(cold=True) for c in r.core[0][0]])
226.79734466500537

sum([c.getArea(cold=False) for c in r.core[0][0]])
225.65518533985468

In reality, the area of the DerivedShape should change as the areas of the other components change.

john-science commented 1 month ago

Okay, so the problem appears to be that the cold=True parameter isn't how we drive DerivedShapes:

https://github.com/terrapower/armi/blob/9dc6760de7da3800cbe98e83066cf5f343e81a6a/armi/reactor/components/__init__.py#L447-L459

The idea is that the DerivedShape is just the liquid left over to fill all the other solid shapes around. So, you can not change the area/volume of a DerivedShape by driving the temperature of the component. You would have to drive the temperature of the parent component.

So, I suppose I would normally like to totally remove the cold=True parameter from that method. But that would break a lot of code, so we can't break the subclass paradigm. It would likewise be hard to have this parameter raise an error, since... which way would raise the error True or False?

Okay, what you prefer would happen?

keckler commented 1 month ago

In my understanding, cold=True effectively means "give me the area as it was defined in the blueprints", or "give me the area at the temperature corresponding to Tinput". This is based on the docstring in the abstract base class: https://github.com/terrapower/armi/blob/9dc6760de7da3800cbe98e83066cf5f343e81a6a/armi/reactor/components/component.py#L609-L618

Since the area of a DerivedShape is determined based on the areas of other components in the block, I would therefore expect the area of a cold DerivedShape component to be calculated using the areas of other components in the block when they are at their cold temperatures. So basically, I would expect that we would first have to loop over all of the other components in the block and determine their cold areas, and then subtract them from the block's area to get the cold area of the DerivedShape.

keckler commented 1 month ago

Also, kind of a sidebar, but I noticed that the various instances of getComponentArea() are all over the place with respect to actually using the cold argument or not, or properly documenting if it is used or not.

This particular instance of getComponentArea() says cold is "Ignored for this component", but that is untrue. cold=True is checked and used: https://github.com/terrapower/armi/blob/9dc6760de7da3800cbe98e83066cf5f343e81a6a/armi/reactor/components/__init__.py#L154-L167

This one does not use the cold argument at all, but does not say so in the docstring: https://github.com/terrapower/armi/blob/9dc6760de7da3800cbe98e83066cf5f343e81a6a/armi/reactor/components/__init__.py#L240-L241