openmc icon indicating copy to clipboard operation
openmc copied to clipboard

Add random ray info to statepoint file

Open spasmann opened this issue 6 months ago • 5 comments

Description

This PR adds some of the basic random ray simulation parameters and summary information to the statepoint file. I tried to copy similar additions but please let me know if things could be organized differently/better!

Checklist

  • [x] I have performed a self-review of my own code
  • [x] I have run clang-format (version 15) on any C++ source files (if applicable)
  • [x] I have followed the style guidelines for Python source files (if applicable)
  • [x] I have made corresponding changes to the documentation (if applicable)
  • [ ] I have added tests that prove my fix is effective or that my feature works (if applicable)

spasmann avatar Jul 14 '25 01:07 spasmann

This is great -- thanks so much @spasmann!

The only potential change that comes to mind might be to have the python statepoint object variables stored in a random_ray dictionary rather than at that top level, so as to mirror how the input settings are handled in the settings.random_ray dictionary. As the random ray implementation grows there are bound to be more settings introduced, so may be nice to keep them in their own dictionary.

If it's a huge pain to change (or you disagree with the dictionary idea in general), let me know! If it's just a matter of free time, let me know and I can give the conversion into a dictionary a go.

jtramm avatar Jul 14 '25 15:07 jtramm

I think that's a good idea @jtramm, see if this looks better.

spasmann avatar Jul 14 '25 16:07 spasmann

@paulromano - that is definitely a valid point. In general it is not good to store the same data twice if it can be avoided.

There is however the case where you are doing a parameter sweep or collecting data for a number of different setting options. We will naturally be saving the statepoints somewhere after each run so we can analyze them later (e.g., load tallies and see how they changed), but may neglect to save a copy of the input XML to go along with each unique statepoint. In that case, you wouldn't know which statepoint belonged to which input setting choice. The fix of course is to save both the input XML and the output statepoint for each case you're running, but it is sort of nice to have the statepoint be fully self descriptive. Removing input settings from the statepoint seems like it would unavoidably lead to some number of statepoints having to be recomputed as users forgot to also save the XML that corresponded to it.

jtramm avatar Jul 18 '25 20:07 jtramm

Another potential issue I suppose is that if users are able to load some settings from the statepoint, they will likely assume that all the settings are in there as well, so may not bother to save the .xml that goes along with their statepoint. If we wanted to switch to storing settings in the .xml alone, might be better to make that jump all at once rather than migrating there over time?

jtramm avatar Jul 18 '25 20:07 jtramm

Regardless of the strategy we use regarding storing settings.xml inputs in the statepoint file, there should probably be a regression test included with this PR, no? @jtramm

yardasol avatar Dec 01 '25 04:12 yardasol