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

Inconsistency between `findAllAxialMeshPoints()` and `getAxialMesh()` #759

Open albeanth opened 2 years ago

albeanth commented 2 years ago

core.findAllAxialMeshPoints() includes 0.0 in its return. https://github.com/terrapower/armi/blob/1990fcd5c77d11797f41d29212b3d17a82ad1ed4/armi/reactor/reactors.py#L1810-L1813

assembly.getAxialMesh() does not https://github.com/terrapower/armi/blob/1990fcd5c77d11797f41d29212b3d17a82ad1ed4/armi/reactor/assemblies.py#L372-L375

This can be confusing in some instances and is an opportunity to improve consistency amongst related methods. An example -- https://github.com/terrapower/armi/blob/1990fcd5c77d11797f41d29212b3d17a82ad1ed4/armi/reactor/converters/geometryConverters.py#L1097-L1104

A fix could be as simple as initializing meshVals with 0.0. One would just need to resolve dependencies on assembly.getAxialMesh https://github.com/terrapower/armi/blob/1990fcd5c77d11797f41d29212b3d17a82ad1ed4/armi/reactor/assemblies.py#L395-L414

onufer commented 2 years ago

setBlockMesh also doesn't expect 0.0. Consistency is important, and @ntouran and others agree we should be 0.0 inclusive to be explicit. Also, bottom is not always guaranteed to be 0.0 (could be negative for some systems)

albeanth commented 2 years ago

A note to my future self of an instance where there is 0.0 inconsistency.

https://github.com/terrapower/armi/blob/20ca6877c2e2221bc3e3521cf9e827cf2d4a4376/armi/reactor/reactors.py#L1692-L1696

john-science commented 3 months ago

@albeanth This ticket didn't get on my radar two years ago.

  1. We should talk about it.
  2. Should we mark this ticket as "bug"?
albeanth commented 3 months ago

@john-science sure, let's chat.

I don't think it's a bug but rather a questionable implementation/design choice that could be improved.