Added parameters to keep all statepoints in transient solves. by TheBEllis · Pull Request #1313 · neams-th-coe/cardinal

TheBEllis · 2026-04-01T14:37:44Z

Addresses issue #1312. Adds two parameters to OpenMCProblemBase, statepoint_directory and keep_transient_statepoint.

statepoint_directory allows the user to set the directory they want the statepoint file to be written to. In OpenMCProblemBase this sets openmc::settings::path_output.

keep_transient_statepoint creates a directory for each timestep's statepoint file to be saved to. If the user has already set statepoint_directory, this value is used as a base directory name, and a suffix is appended to indicate the timestep.

I've added a couple of tests in, just to check the statepoints get output to the expected directories.

moosebuild · 2026-04-01T15:01:27Z

Job Documentation, step Sync to remote on 2e8bfa7 wanted to post the following:

View the site here

This comment will be updated on new commits.

aprilnovak · 2026-04-01T15:54:14Z

@TheBEllis can you rebase your branch off devel? It looks like there's a bunch of other commits coming along for the ride.

…to OpenMCBaseProblem

…et default base name for transient statepoint dirs

… sets a statepoint_directory

…point parameters

…e#1312

… exists before creating it

TheBEllis · 2026-04-01T16:01:55Z

Whoops my bad, should be rebased now!

nuclearkevin

Thanks for opening this PR @TheBEllis! I have one question regarding the design and I think another test should be added to catch an edge case, but otherwise I don't have any major complaints.

nuclearkevin · 2026-04-01T19:28:31Z

+[Executioner]
+  type = Transient
+  num_steps = 2 
+  batches = 50 # this number needs to match the value in settings.xml for the purpose of the test


batches belongs in the [Problem] block.

nuclearkevin · 2026-04-01T19:29:15Z

+[Executioner]
+  type = Transient
+  num_steps = 2 
+  batches = 50 # this number needs to match the value in settings.xml for the purpose of the test


batches belongs in the [Problem] block.

nuclearkevin · 2026-04-01T19:38:27Z

+  if (isParamSetByUser("statepoint_directory") && !_keep_transient_statepoint)
+  {
+    openmc::settings::path_output = formattedOutputPath(_statepoint_directory);
+    if (!std::filesystem::is_directory(openmc::settings::path_output))


Are we catching this if statement with a test? From what I can see, none of the tests try to create a statepoint directory with the same name as a file.

I've added a RunException for this exact scenario, let me know if you think that's enough or whether I should be catching it in code:)

This works for me!

nuclearkevin · 2026-04-04T16:26:39Z

It appears as if the include statements are still outside of the #ifdef ENABLE_OPENMC guard, which is why the NekRS test is failing.

~~I don't know off the top of my head where the cascading OpenMC/DAGMC test failures are coming from.~~ The test timeouts appear to be an issue on the CIVET side of things as they're common to all PRs.

…rio when user defined output path has the same name as existing file

…so that potentially non existant directories can be compared

moosebuild · 2026-04-07T16:56:45Z

Job Coverage, step Generate coverage on 2e8bfa7 wanted to post the following:

Coverage

Inconsistent report tags were found between the head and base reports.
This can happen when reports are missing from either the head or the base.

Inconsistent tags:
nekrs
Full coverage report

This comment will be updated on new commits.

nuclearkevin

A few more comments from me, but I think this is almost ready to go.

Terribly sorry for the inconvenience dealing with the regression suite being non-deterministic - we're trying to figure out what's going on but can't find any patterns.

nuclearkevin · 2026-04-07T23:22:26Z

+  [check_statepoint_exists_in_directory_w_existing_filenmae]
+    type = RunException
+    input = openmc_custom_dir_filename.i
+    expect_err = 'filesystem error: cannot create directory: File exists'


Does this trace back to OpenMCCellAverageProblem through a mooseError()? if not, it will be difficult for the user to debug the error message.

I've moved the logic around a bit so that a mooseError() is thrown now. There are some slightly annoying lines in there, mostly related to removing trailing "/". The value passed to openmc::settings::path_output must contain a trailing "/", but when checking is a file with the same name exists using is_regular_file(), this has to be removed.

…ce to user

nuclearkevin

LGTM!

moosebuild · 2026-04-08T15:41:26Z

Job Test OpenMC on 2e8bfa7 : invalidated by @nuclearkevin

nuclearkevin · 2026-04-08T15:42:20Z

I'll rerun the tests until they pass. The random RunException failures are unrelated.

moosebuild · 2026-04-08T16:04:28Z

Job Test DagMC on 2e8bfa7 : invalidated by @nuclearkevin

moosebuild · 2026-04-08T16:04:45Z

Job Coverage on 2e8bfa7 : invalidated by @nuclearkevin

moosebuild · 2026-04-08T16:05:10Z

Job Coverage on 2e8bfa7 : invalidated by @nuclearkevin

aprilnovak · 2026-04-08T17:39:21Z

+  const std::string & _statepoint_directory;
+
+  /// Parameter determines whether statepoints from all timesteps should be saved in separtate directories to avoid them being overwritten
+  const bool _keep_transient_statepoint;


Suggested change

const bool _keep_transient_statepoint;

const bool & _keep_transient_statepoint;

capturing by reference is recommended by MOOSE convention when possible because it'll be compatible with controls and other systems

aprilnovak · 2026-04-08T17:43:36Z

+  params.addParam<bool>("keep_transient_statepoint",
+                        false,
+                        "Whether or not statepoints from all timesteps should be kept, and written "
+                        "to seperate directories.");


Suggested change

"to seperate directories.");

"to separate directories.");

aprilnovak · 2026-04-08T17:43:36Z

+  const std::string & _statepoint_directory;
+
+  /// Parameter determines whether statepoints from all timesteps should be saved in separtate directories to avoid them being overwritten
+  const bool _keep_transient_statepoint;


capturing by reference is recommended by MOOSE convention when possible because it'll be compatible with controls and other systems

aprilnovak · 2026-04-08T17:50:20Z

+    requirement = "A statepoint file must be written to the parameter defined directory when the batches parameter matches the batches "
+                  "in settings.xml and they are both specified."
+    capabilities = 'openmc'
+  []


please make sure each test has a issues and design parameter

TheBEllis added 8 commits April 1, 2026 16:55

Added keep_transient_statepoint and statepoint_path input parameters …

828d464

…to OpenMCBaseProblem

added check to see if user sets statepoint_directory to './', if so s…

02825d4

…et default base name for transient statepoint dirs

Added tests for when user sets keep_transient_statepoint to true, and…

12070c9

… sets a statepoint_directory

Updated doc strings for statepoint_directory and keep_transient_state…

4b679ca

…point parameters

Added test for custom user output directory

911d3ed

Made sure final path is lexically normal, addresses issue neams-th-co…

4d1a188

…e#1312

Removed added includes, addresses neams-th-coe#1312

9dde623

Added check to see if openmc::settings::path_output directory already…

cfd7c4c

… exists before creating it

TheBEllis force-pushed the statepoint_timestamps branch from f241885 to 4367813 Compare April 1, 2026 15:55

aprilnovak requested a review from nuclearkevin April 1, 2026 18:25

nuclearkevin suggested changes Apr 1, 2026

View reviewed changes

Added xml files to statepoint transient tests

a9e228b

TheBEllis force-pushed the statepoint_timestamps branch from 4367813 to a9e228b Compare April 2, 2026 09:27

TheBEllis added 2 commits April 2, 2026 11:46

Addressed PR comments

6d89d2c

added doc comments, removed unnecessary canonically_weak call

f4642de

TheBEllis force-pushed the statepoint_timestamps branch from 734b297 to 9c36899 Compare April 2, 2026 15:47

Fixed wrong retrieval of absolute file path, and added test for scena…

1b8b26a

…rio when user defined output path has the same name as existing file

TheBEllis force-pushed the statepoint_timestamps branch from d78c475 to 1b8b26a Compare April 7, 2026 12:48

TheBEllis added 2 commits April 7, 2026 15:53

Switched filesystem::equivalent call to weakly_canonical comparison, …

e585cac

…so that potentially non existant directories can be compared

Changed transient statepoint tests design OpenMCCellAverageProblem

55beaf6

nuclearkevin suggested changes Apr 7, 2026

View reviewed changes

Added MOOSE errors so that create directory errors provide useful tra…

2e8bfa7

…ce to user

nuclearkevin approved these changes Apr 8, 2026

View reviewed changes

aprilnovak reviewed Apr 8, 2026

View reviewed changes

	const bool _keep_transient_statepoint;
	const bool & _keep_transient_statepoint;

Conversation

TheBEllis commented Apr 1, 2026

Uh oh!

moosebuild commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aprilnovak commented Apr 1, 2026

Uh oh!

TheBEllis commented Apr 1, 2026

Uh oh!

nuclearkevin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nuclearkevin commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moosebuild commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage

Uh oh!

nuclearkevin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nuclearkevin left a comment

Choose a reason for hiding this comment

Uh oh!

moosebuild commented Apr 8, 2026

Uh oh!

nuclearkevin commented Apr 8, 2026

Uh oh!

moosebuild commented Apr 8, 2026

Uh oh!

moosebuild commented Apr 8, 2026

Uh oh!

moosebuild commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

moosebuild commented Apr 1, 2026 •

edited

Loading

nuclearkevin commented Apr 4, 2026 •

edited

Loading

moosebuild commented Apr 7, 2026 •

edited

Loading