HexBlock.autoCreateSpatialGrids raises lots of spurious exceptions

[drewj-tp]

It's not uncommon for me to see messages like

[warn] Could not create a spatialGrid for block BLOCK, multiplicities are not 1 or N they are {1}

which, what? I read this as "multiplicities are not 1 they are 1" which enforces a sense of distrust in warning messages.

Tracking this down, we try/except Block.autoCreateSpatialGrids when creating a block in blueprints

https://github.com/terrapower/armi/blob/f1f3dbea05c2564404a03d8e053f72c2af15daa9/armi/reactor/blueprints/blockBlueprint.py#L243-L247

and the HexBlock implementation throws this error

https://github.com/terrapower/armi/blob/f1f3dbea05c2564404a03d8e053f72c2af15daa9/armi/reactor/blocks.py#L2353-L2358

The error appears to be triggered because we don't have two multiplicities. Which I guess would be what you'd expect an assembly with pins to have: single multiplicity for a duct or other bounding component, and then a multiplicity > 1 for pin-like components.

Remedy

If there is only a single multiplicity, and it's close to one, do not attempt to make a spatial grid on the block. This feels acceptable. There's nothing here to make a spatial grid so we won't. It's not that we cannot make a spatial grid (e.g., invalid number of pins), just that it doesn't make sense to create on here. Maybe the autocreate method raises an exception that is silently discarded in the block blueprint?

[keckler]

+1 on this issue. I see this warning ~10 bazillion times whenever I run, and the warning message is not very useful.

I would also say that the warning is not entirely justified, considering that nobody guaranteed the user that a spatial grid would be created on their block.

I guess this message feels more like a "extra" or "debug" kind of thing, to me.

[drewj-tp]

I would also say that the warning is not entirely justified, considering that nobody guaranteed the user that a spatial grid would be created on their block.

Totally get the confusion, especially for things that don't make sense to have grids.

Surprise surprise, there's a setting, defaulting to True, that controls this

https://github.com/terrapower/armi/blob/f1f3dbea05c2564404a03d8e053f72c2af15daa9/armi/settings/fwSettings/globalSettings.py#L832-L839

[john-science]

Well, I like an easy one.

I think, perhaps, we should:

Make the default for autoGenerateBlockGrids / CONF_BLOCK_AUTO_GRID False.

A quick review tells me that no one anywhere in the world uses this setting. So, if it's only job is to turn on this log statement, then fine the log statement can stay info level, IF the setting is off by default.

Alternatively:

1. We could remove the setting, it's nigh useless.
2. We could just move this log statement to debug.

Thoughts?

[keckler]

Huh, didn't know about that setting! In light of that, I guess the warning message makes a little bit more sense then.

I think that the assigning of auto-grids to the blocks is useful, ,so even if the default was changed for the setting, I would just turn it back on. And in light of this behavior being controlled by a setting, it feels a bit strange to bury the warning behind a debug print.

Hmmm...

Maybe the answer here is to have a runLog.warning that says something like "Warning: 500 blocks could not auto create a grid", and then separate runLog.extra messages that are for the specific blocks and the reasoning that the grid could not be created.

No matter, the message statement that Drew originally posted is cryptic and should be made more instructive for the user.

[drewj-tp]

A quick review tells me that no one anywhere in the world uses this setting

I think the answer is sneakier. No one turns the setting off or on explicitly, but I'm 100% certain people rely on the functionality this setting controls. We use it to get a spatial grid so we can have Block.getPinCoordinates be meaningful.

The alternative to not autosetting grids is to have a grids definition for every block in your reactor. Which would be extremely cumbersome. In simple cases, where you have the same pin-like thing at every lattice site in your reactor, and you have a mult that would fill an entire hexagonal grid, the auto creation of spatial grids is a huge time saver.

Maybe the answer here is to have a runLog.warning that says something like "Warning: 500 blocks could not auto create a grid"

That's a step in the right direction I think. Slap a single=True and `label="Cannot auto create spatial grid".

I would wager that 500 of those 500 blocks do not need a spatial grid. Because they are just blocks with single mult components. I would say there should be no warning and no grid created if mult == {1, }. That's not a thing where it makes sense to have a grid, and it's easy to detect

[john-science]

That's a step in the right direction I think. Slap a single=True and `label="Cannot auto create spatial grid".

Yeah, that's a good idea.

[keckler]

Note that the setting mentioned above (CONF_BLOCK_AUTO_GRID, otherwise known as autoGenerateBlockGrids) was removed here: https://github.com/terrapower/armi/pull/1947/files#r1797329551

[keckler]

One way to potentially cut down on the messages would be to move this line up to the top of the autoCreateSpatialGrids() method: https://github.com/terrapower/armi/blob/1920ff0ecb146a050c1c2f0a199cf620834728b2/armi/reactor/blocks.py#L2322

That method is pretty inefficient, in the sense that it will do almost all of the work before even checking if the block needs a spatial grid. In cases where the block already has one, we can skip all the work entirely.

[john-science]

Honestly, I have not fully tested Chris' idea to move the if statement up. The original version of the code I was working from had this if statement so far down the method for good reason.

But it certainly looks like this check could be done at the top of the method now, with no problems.

[alexhjames]

This same Block.autoCreateSpatialGrids method is likely the source of these warning messages as well:

[warn] Cannot get pin pitch in BLOCK because it does not have a wire and a clad

It is generating it from this line: https://github.com/terrapower/armi/blob/2fc524aecbf32e7270e6348fd4665447bd47d645/armi/reactor/blocks.py#L2227-L2232

I suspect, related to the poor handling of blocks that have a multiplicity of 1, it is also poorly handling blocks that don't have pins. Seems like it should get fixed in the same refactor.

[john-science]

Thanks to @alexhjames for the info!

I think the two test scenarios are pretty basic, it's 1) have a block that has a component with mult=2 and a component with mult=1 and make sure the block doesn't include clad or wire components and 2) have a block that only has components with mult=1.

[keckler]

Thanks to @alexhjames for the info!

I think the two test scenarios are pretty basic, it's 1) have a block that has a component with mult=2 and a component with mult=1 and make sure the block doesn't include clad or wire components and 2) have a block that only has components with mult=1.

I do not follow. Can you elaborate the two scenarios?

[alexhjames]

Scenario 1 (block with a component with mult=2 - support structures in this case, and a component with mult=1 - duct in this case, no clad or wire components):

moveable top plate load pad:
    support structures:
      flags: structure
      shape: Circle
      material: HT9
      Tinput: 20.0
      Thot: 500.0
      mult: 2
      od: 14.9
      id: 13.5
    duct:
      <<: *duct
      Thot: 500.0
    intercoolant:
      <<: *intercoolant
      Tinput: 500.0
      Thot: 500.0
    coolant:
      <<: *coolant
      Tinput: 500.0
      Thot: 500.0

Scenario 2 (block with only components that have mult=1):

grid plate:
    grid:
      shape: Hexagon
      material: SS316
      Tinput: 20.0
      Thot: 400.0
      ip: 7.00
      mult: 1
      op: 16
    intercoolant:
      <<: *intercoolant
      Tinput: 400.0
      Thot: 400.0
      ip: grid.op
    coolant:
      <<: *coolant
      Tinput: 400.0
      Thot: 400.0