Tally and Domain Filter Editor UOs #923

pshriwise · 2024-07-08T20:40:01Z

This adds a UserObject for editing OpenMC Tally and some Filter objects. The order of execution in OpenMCProblemBase should ensure they can be used in conjunction to create custom, controllable tallies in a Cardinal input file (for the set of filters supported, see below).

OpenMCTallyEditor

This UO can be used to either edit an existing tally object or create a new one via the Cardinal input file. A tally's scores, nuclides, filters (set by ID) can all be specified in the input file. These parameters can also be controlled using the MOOSE control system. The tally's multiply_density attribute, governing whether or not tally results are multiplied by atom density, can be set as well, but as this is used less often it is set to false by default. multiply_density can also be controlled via the MOOSE control system. Other parameters are required.

If the create_tally parameter of an OpenMCTallyEditor object is true and a tally already exists with the specified ID, a warning is shown. If create_tally is false and a tally with the specified ID doesn't exist, an error is raised.

The tally UO provides functionality that can replace the OpenMCTallyNuclides and I've done so in this PR. I'm not sure that deprecation of the existing OpenMCTallyNuclides object is necessary given its short lifetime, but I'm happy to provide that here.

OpenMCDomainFilterEditor

The domain filter editor allows for the creation of tally filters and modification of their associated bins (by cell/material/universe/mesh ID). Like the tally properties above, filter bins can be set in the input file and modified between OpenMC executions as part of the MOOSE control system.

The supported filter types for this UO include: CellFilter, MaterialFilter, UniverseFilter, and MeshFilter. These are all spatial/domain filters that have the same generic bin structure and are important for depletion/activation workflows that may be attached to Cardinal multiphysics problems.

The filter editor UO is complicated by the necessity of a filter type. Additional checks have been added that ensure:

if create_filter is true and a tally filter with the specified ID already exists:
- when filter_type parameter is set, the specified filter_type is checked against the existing Filter's type and an error is raised on mismatch
- if filter_type is not set or the types match, a warning is shown
if create_filter is false and a filter with a matching ID exists:
- when filter_type parameter is set, the specified filter_type is checked against the existing Filter's type and an error is raised on mismatch
  - if filter_type is not set, no additional check is performed other than to check that the filter is in the set of types supported by OpenMCDomainFilter

TODO:

Complete TallyEditor testing
Complete DomainFilterEditor testing
tally editor docs
domain filter editor docs

moosebuild · 2024-07-08T21:26:42Z

Job Documentation on 107d06f wanted to post the following:

View the site here

This comment will be updated on new commits.

moosebuild · 2024-07-08T22:40:53Z

Job Coverage on 107d06f wanted to post the following:

Coverage

	5a5190	#923 107d06
	Total	Total	+/-	New
Rate	93.39%	93.32%	-0.08%	90.91%
Hits	7436	7594	+158	200
Misses	526	544	+18	20

Diff coverage report

Full coverage report

This comment will be updated on new commits.

aprilnovak · 2024-07-11T21:57:19Z

src/userobjects/OpenMCDomainFilterEditor.C

+  InputParameters params = OpenMCUserObject::validParams();
+  params.addParam<bool>("create_filter", false, "Whether to create the tally if it doesn't exist");
+  params.addRequiredParam<int32_t>("filter_id", "The ID of the filter to modify");
+  params.addParam<std::string>("filter_type", "", "The type of filter to create");


Let's make this a MooseEnum -- it will then render the options automatically in the documentation. If you haven't used that type before, you can follow examples in the CardinalEnums.C/CardinalEnums.h files and how MooseEnum's are used in OpenMCCellAverageProblem.

doc/content/source/userobjects/OpenMCDomainFilterEditor.md

include/userobjects/OpenMCDomainFilterEditor.h

aprilnovak · 2024-07-11T22:00:08Z

include/userobjects/OpenMCDomainFilterEditor.h

+  bool _first_execution{true};
+  int32_t _filter_id;
+  std::string _filter_type;
+  const std::set<std::string> _allowed_types{"cell", "universe", "material", "mesh"};


Convention is to use a MooseEnum if possible

include/userobjects/OpenMCTallyEditor.h

include/userobjects/OpenMCUserObject.h

aprilnovak · 2024-07-11T22:04:35Z

src/userobjects/OpenMCDomainFilterEditor.C

+void
+OpenMCDomainFilterEditor::initialize()
+{
+  bool create_filter = getParam<bool>("create_filter");


Is there a reason this object isn't fetched from the member initialization list?

It's used often in this method, so I assigned a local variable to decrease code verbosity.

src/userobjects/OpenMCDomainFilterEditor.C

…lly and filter editors

nuclearkevin

After going over the changes, I don't think there are any clashes between these userobjects and the tally system when xml tallies/filters are modified. I also don't believe this will cause any issues with the filter system coming in #951 since those are thin wrappers that return values stored in OpenMC filter objects.

I can see issues arising if the user attempts to modify tallies/filters added through the [Problem/Tallies] block since the changes made by these userobjects don't get propagated to either CellTally or MeshTally wrapper classes. It might be worth restricting the use of both userobjects so they only modify xml tallies or create new ones, unless you want to find a way to propagate changes in the filter/tallies to CellTally and MeshTally.

nuclearkevin · 2024-08-21T19:33:37Z

src/userobjects/OpenMCDomainFilterEditor.C

+  params.declareControllable("bins");
+  params.addClassDescription("A UserObject for creating and managing OpenMC domain tally filters");
+  return params;
+}


The ids of the OpenMC objects created by Cardinal through the [Problem/Tallies] block are opaque to the user, so if the user wants to modify filters added by mapped tallies they'd need to guess and check until they found the right id.

If mapped filters need to be changed (distribcell for CellTally and mesh for MeshTally, it would make sense to add an input parameter for the tally objects name in the input file and use it to fetch the associated ids. Same comment applies to the OpenMCTallyEditor class.

Very true @nuclearkevin. I wasn't intending these for use with mapped tallies. Unless there's a use case I'm not seeing, I don't think we'd want users controlling those given the amount of programmatic mapping we do in the OpenMCCellAverageProblem. For now, I think it's best if we assume these are limited to unmapped tallies.

I could also add some code to preclude their use for mapped Cardinal tallies.

I also can't think of any reasons why OpenMCTallyEditor/OpenMCDomainFilterEditor should be able to modify mapped tallies/filters, so I'm good with restricting their use. It would probably be best to perform a programmatic check when constructing / initializing the userobjects (since they're added after tallies during initialization) to make sure the ids supplied by the users don't conflict with any of the mapped tally/filter ids just to be sure.

If the plan is to avoid the modification of mapped tallies the docs would need to be modified as they explicitly state that this is one of the use cases for these userobjects.

Agreed! I think that's a good plan. I'll implement a check that ensures the editor IDs and mapped IDs aren't overlapping. I'll look to the documentation too once I've done that. 👍🏻

nuclearkevin · 2024-08-21T19:40:24Z

src/userobjects/OpenMCDomainFilterEditor.C

+}
+
+void
+OpenMCDomainFilterEditor::execute()


Do these changes propagate to the data structures in either CellTally or MeshTally when the filter editor is modifying a mapped tally? We initialize a good number of temporary data structures for normalization and outputting to the MooseMesh in OpenMCCellAverageProblem based on variables stored in CellTally/MeshTally (not the tallies or filters they wrap). If they aren't updated when the wrapped tallies are modified it could cause unexpected behaviour.

I'll check back on this depending on the discussion above

#923 (comment)

nuclearkevin · 2024-08-21T19:49:11Z

src/userobjects/OpenMCTallyEditor.C

+  openmc::Tally * tally = openmc::model::tallies[tally_index()].get();
+
+  std::vector<std::string> scores = getParam<std::vector<std::string>>("scores");
+  if (scores.size() > 0)


It doesn't look like modifications to tally scores propagates to the tally objects for the case where mapped tallies are modified by the OpenMCTallyEditor. A vector of scores in each tally object is used to initialize temporary data structures for normalization. If these stored tally scores aren't updated, some scores won't get normalized and other scores may be normalized incorrectly if the controlled scores parameter changes the order.

Action on this will depend on the discussion above

#923 (comment)

pshriwise · 2024-08-22T14:23:09Z

@aprilnovak I'm seeing similar Nek-related test failures on other PRs and the changes here don't seem relevant (correct me if I'm wront) so I'm not going to dig into that right away.

aprilnovak · 2024-08-22T15:35:43Z

You can ignore the container failures -- the Nek ones are all passing, the JIT seems to have gotten slower so there's just some timeouts but they are fine.

@loganharbour, anything special we should be doing about the container failures? I believe this is also related to the failure on #952

nuclearkevin · 2024-08-22T19:14:21Z

doc/content/source/problems/OpenMCCellAverageProblem.md

+  - [OpenMCTallyEditor](https://cardinal.cels.anl.gov/source/userobjects/OpenMCTallyEditor.html)
+  - [OpenMCDomainFilterEditor](https://cardinal.cels.anl.gov/source/userobjects/OpenMCTallyEditor.html)
+
+These objects provide online control of tally and filter parameters, respectively. These objects can be used to interact with tallies and/or filters that are mapped in Cardinal as part of the `OpenMCCellAverageProblem` or are present in a `tallies.xml` file for the problem being run (unmapped tallies). These objects can be used to create tallies as well -- in this case the new objects will **not be mapped** to the mesh mirror.


I would recommend revising this section of the documentation to make it clear to the user that mapped tallies cannot be modified by either OpenMCTallyEditor or OpenMCDomainFilterEditor.

moosebuild · 2024-09-19T20:57:38Z

Job Precheck on b25c149 wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/cardinal/docs/PRs/923/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 5a5190e6b3e40f7c5f9f0288b0055ec38e0d1c6b

pshriwise changed the title ~~Tally uos~~ Tally and Domain Filter Editor UOs Jul 8, 2024

aprilnovak reviewed Jul 11, 2024

View reviewed changes

doc/content/source/userobjects/OpenMCDomainFilterEditor.md Outdated Show resolved Hide resolved

aprilnovak reviewed Jul 11, 2024

View reviewed changes

include/userobjects/OpenMCDomainFilterEditor.h Outdated Show resolved Hide resolved

aprilnovak reviewed Jul 11, 2024

View reviewed changes

include/userobjects/OpenMCTallyEditor.h Outdated Show resolved Hide resolved

aprilnovak reviewed Jul 11, 2024

View reviewed changes

include/userobjects/OpenMCUserObject.h Outdated Show resolved Hide resolved

aprilnovak reviewed Jul 11, 2024

View reviewed changes

src/userobjects/OpenMCDomainFilterEditor.C Outdated Show resolved Hide resolved

aprilnovak mentioned this pull request Jul 24, 2024

Addition of a tally system and [Tallies] block #931

Merged

pshriwise added 18 commits August 21, 2024 13:11

A start to creating tallies with UOs

56d86f6

Initial cut at at UserObject for tally generation

ba4f527

Adding tally generator uo in a new spot

1ca6ee9

Moving to reliance on a tally generator user object

4a1cf4f

Moving to rely on tally generator user object

e260d8c

push for logan

003e4a9

Updating tally generator UOs to handle updates as well.

31fe02f

Name change. QOL funcs

e81626b

One more name change

54d5181

Adding a domain filter editor

8ef4c2e

Updating openmc_xml test files for now

e2f571c

Including a parameter for multiplication by atom density

6512da4

Removing tally generator action

19c8ced

Reverting changes to the openmc_xml test

a283225

Updating copywrite year.

9305b5e

Added error checking. Changing timing of tally ID check.

2c2d322

Updates to tests to use the tally editor instead of tally nuclides

371440c

Some quick self-review

996544c

pshriwise added 14 commits August 21, 2024 13:14

New test for clashing type with existing filter.

3747b05

CI trigger

37513d7

Test input file formatting

adf7948

Source formatting

ced4b2a

Correcting filter editor UO class name in docs

cd6a8f7

Updating comment on allowed filter types

cd6904c

Moving _first_execution into base class

06c9caa

Factoring out first_execution method into base object

3a1fe15

Use UO console

89daf68

Changing filter type attribute to an enumerator

bcdf830

EOL for cardinal enums src file

9ede8c3

CI trigger

b5533af

CI trigger

2a37b87

Updating expected behavior and test files

7f3a512

pshriwise force-pushed the tally_uos branch from 35914d7 to 7f3a512 Compare August 21, 2024 18:20

aprilnovak requested a review from nuclearkevin August 21, 2024 18:24

pshriwise added 2 commits August 21, 2024 13:43

Breaking up the current tallies section a bit. Adding a section on ta…

9e7b3b6

…lly and filter editors

Remove commented section

9646b61

nuclearkevin reviewed Aug 21, 2024

View reviewed changes

pshriwise added 2 commits August 21, 2024 15:11

Style guide fix

6f16ad3

Style fix

8ea3dbc

nuclearkevin reviewed Aug 22, 2024

View reviewed changes

pshriwise added 3 commits September 19, 2024 11:20

Clarifying use case for Tally/Filter editors.

50715ec

Removing unused OpenMCDomainFilter::_allowed_types attribute

c578788

Ensuring that TallyEditor IDs don't overlap with Cardinal-mapped tallies

b25c149

pshriwise added 2 commits September 19, 2024 16:02

Formatting

c0d8a83

Adding new test input file

107d06f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tally and Domain Filter Editor UOs #923

Tally and Domain Filter Editor UOs #923

pshriwise commented Jul 8, 2024 •

edited

Loading

moosebuild commented Jul 8, 2024 •

edited

Loading

moosebuild commented Jul 8, 2024 •

edited

Loading

aprilnovak Jul 11, 2024

aprilnovak Jul 11, 2024

aprilnovak Jul 11, 2024

pshriwise Jul 12, 2024 •

edited

Loading

nuclearkevin left a comment •

edited

Loading

nuclearkevin Aug 21, 2024 •

edited

Loading

pshriwise Aug 22, 2024

nuclearkevin Aug 22, 2024

pshriwise Aug 26, 2024 •

edited

Loading

nuclearkevin Aug 21, 2024

pshriwise Aug 22, 2024

nuclearkevin Aug 21, 2024

pshriwise Aug 22, 2024

pshriwise commented Aug 22, 2024

aprilnovak commented Aug 22, 2024

nuclearkevin Aug 22, 2024

moosebuild commented Sep 19, 2024

Tally and Domain Filter Editor UOs #923

Are you sure you want to change the base?

Tally and Domain Filter Editor UOs #923

Conversation

pshriwise commented Jul 8, 2024 • edited Loading

OpenMCTallyEditor

OpenMCDomainFilterEditor

moosebuild commented Jul 8, 2024 • edited Loading

moosebuild commented Jul 8, 2024 • edited Loading

Coverage

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshriwise Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

nuclearkevin left a comment • edited Loading

Choose a reason for hiding this comment

nuclearkevin Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshriwise Aug 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshriwise commented Aug 22, 2024

aprilnovak commented Aug 22, 2024

Choose a reason for hiding this comment

moosebuild commented Sep 19, 2024

pshriwise commented Jul 8, 2024 •

edited

Loading

moosebuild commented Jul 8, 2024 •

edited

Loading

moosebuild commented Jul 8, 2024 •

edited

Loading

pshriwise Jul 12, 2024 •

edited

Loading

nuclearkevin left a comment •

edited

Loading

nuclearkevin Aug 21, 2024 •

edited

Loading

pshriwise Aug 26, 2024 •

edited

Loading