Assorted Questions and Thoughts

[john-science]

Here are some assorted questions and thoughts, for your perusal @AidanMcDonald .

1. Can you explain that one-liner ARMI change to me?

   • Sorry. I obviously see the commit message "Dont check if DerivedShape components are enveloped by other components", but I was hoping for a little more detail. I'm just trying to wrap my head around this change, generally. So I can verify that it doesn't affect anyone else in the ARMI ecosystem.

2. embedded functions work, but are nearly impossible to unit test and I think if you end up with one really long method with multiple embedded functions, that's harder to read than three little functions:

3. Maybe we should centralize all the plotting tools in this repo.

4. How else could we save this data? A YAML file as a Python package "resource"?

5. What do you think of combining the openmcdemo directory and the armicontrib/armiopenmc directory?

   • Is there any reason we really need two separate namespaces in this repo? Perhaps this is just left over from the old version of the repo?
   • We could put everything under a directory called "armiopenmc", so you don't have to change your imports. Or... what do you like better?

6. I don't love relative imports myself. I think they can be troublesome. I won't die on that hill.

7. The OpenMCWriter.writeGeometry() method is 650 lines long and has a bunch of embedded methods. I think that's a touch long.

8. How could we reorganize to the OpenMCWriter so it has fewer "if this geometry" statements?

   • There are various options. It's just a little hard to read write now. (I mean, well, for me.)

9. Where did these XML files come from?

   • Sorry, I might be blind. But we just moved ARMI from XML to YAML in the past year, and I want to know if I missed something.

[AidanMcDonald]

1. Good news, apparently that one-line armi change is no longer necessary. I use MultiComponentMerger to blend any helix shaped components into the coolant since openmc can't support helixes and was getting an error in the _verifyExpansion step when the solvent was a derivedShape. But I just tried the case that was breaking with the most recent ARMI and it worked fine. Either the issue got fixed or more likely I was calling it incorrectly before.

2/3/4. These issues are artifacts of the lazy way I copied some code from the dif3d plugin to get the nice flux spectra plots. I forgot I did it this way; I'll clean it up.

5. This is definitely just left over from the old repo. Combining the two directories makes a lot of sense.

6. Easy change, done.

7/8. I had the same thought and did a reorganization of the code, but I apparently forgot to push that change. It's up now. It's still convoluted but hopefully a bit more readable.

9. Those xml files are the openmc input files written by OpenMCWriter. They have unfortunately non-descriptive names and having mixed yaml and xml is annoying, but openmc looks for those specific names when it runs so I think we have to live with it.

[john-science]

Awesome!

[john-science]

Do I remember you had to adjust the C5G7 blueprints to get your case running correctly?

armicontrib-openmc/openmcdemo/cli/runC5G7.py

Line 88 in d87aa09

cs = caseSettings.Settings(os.path.join("c5g7-settings.yaml"))

This looks like you used exactly what is (now) in ARMI. If you modified, will the modified versions go in this repo?

Thanks!

[AidanMcDonald]

That's exactly right. The modified (multigroup) version is at openmcdemo/cases/c5g7Multigroup/c5g7-settings.yaml (currently in the multigroupMode branch). The runC5G7 entry point works on both the cases.

[john-science]

Ah! It's just not in the main branch yet.

Okay, cool.

No rush. I was looking at moving the C5G7 YAML files in ARMI to a sliiiiightly different folder. But you're not dependent on where these exactly live in ARMI, so it looks like I'm free.

Thanks!