search

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
220 stars 87 forks source link

GridBlueprints decay to `gridContents` mode when constructed #360

Closed youngmit closed 2 years ago

youngmit commented 3 years ago

GridBlueprint allows for specifying object layout via either grid contents or lattice map on the YAML side. These both map to the gridContents and latticeMap yamlize attributes, respectively. When we go to construct the Grid object, we read latticeMap contents into gridContents, which is then treated as the canonical representation. This has the undesirable consequence that if one reads a GridBlueprint that specifies layout with latticeMap, then constructs the Grid, then writes the blueprint back out, both the grid contents and lattice map sections would be written out. Ideally, the blueprint would somehow either remember which was originally specified, or come up with some rules about when contents get written instead of a lattice map, or otherwise store the canonical contents into some non-yaml attribute instance attribute.

john-science commented 3 years ago

@youngmit Please forgive my rudimentary questions.

  1. Where does the writing back out happen?
  2. Does "lattice map" just mean these ascii-art-style diagrams, as opposed to using numbers to define positions?\
youngmit commented 3 years ago

No worries! If it's not clear in the docs, then that's our bad. As with most yamlize-based stuff the loading/dumping is cloaked in mystery and superstition 🧙‍♂️.

  1. technically anywhere you want, just by calling the dump(). Example of where we actually do this is here: https://github.com/terrapower/armi/blob/09c26c7bde2ca43bdcf71b80948c9bfc283808d3/armi/utils/gridEditor.py#L1521 (yikes on the type(object).dump(); did I write that? 😬). This is actually one of the main places where this issue comes up; if someone opens a grid blueprint in the editor (which started with the lattice map style), does some tweaking, then saves it back out, the resulting blueprints will have the old lattice map, and the new contents in "grid contents".
  2. Yes
davis8dd commented 2 years ago

Hello, now that this issue has a PR merged against it should this issue be closed?

youngmit commented 2 years ago

@davis8dd I think you're right. @john-science happy to close this one?

I assume you're combing for good first issues?

davis8dd commented 2 years ago

I assume you're combing for good first issues?

Yes.

youngmit commented 2 years ago

I assume you're combing for good first issues?

Yes.

I just created #436 , which is actually pretty related to this problem, and might make for a good issue for you. Essentially, we have these "asciimap" classes that we use to map from our text input files to our internal Grids system and back out again. The classes are pretty complicated, and have always been a pain to debug/extend, and we could use some help cleaning them up, or even a clean-sheet re-implementation.