add continue feature for depletion by lewisgross1296 · Pull Request #3272 · openmc-dev/openmc

lewisgross1296 · 2025-01-22T03:20:14Z

Description

This PR closes #2871. To summarize the motivation, I'd like depletion restarts to be a bit easier for users. If you set up a depletion simulation that doesn't end up getting run all the way (think hitting max wall time on an HPC or some other interrupt), the burden then falls to the user to correctly determine what time steps were not run and to create a new script that picks up where the other one left off. In my own experience, it is easy to make a mistake and re-run a simulation that is not what I desired.

All we do here is add a continue_timesteps flag (default to False) that adds a new logic block to the Integrator class. It would be useful in the above case and requires users to provide the Integrator with a set of timesteps and one of power/power_density/source_rate that matches what already exists in a prev_results passed to the Opperator.

With this flag, the depletion restart python script can now be exactly the same as the initial script, just with a continue_timesteps = True flag and a prev_results object loaded from a depletion_statepoint.h5 passed to the Operator. It could contain more timesteps at the end too, but the only requirement is that the data matches what is in the Operator provided to the Integrator for all the existing steps in the results.

This PR does not eliminate any of the existing capability of the depletion API, so all the old use cases/syntax for restarts remain. The flag is optional and defaults to False so no one will need to update past scripts with this change.

Checklist

I have performed a self-review of my own code
~~I have run clang-format (version 15) on any C++ source files (if applicable)~~
I have followed the style guidelines for Python source files (if applicable)
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)

lewisgross1296

Looking for input on this line (636)

dean-krueger

Just one fairly minor comment.

connoramoreno

Re-suggesting my comment from before since it was outdated by the commit of Edgar's suggestion. Might need to apply proper formatting for OpenMC, if applicable (should be black-compliant)

lewisgross1296 · 2025-01-28T21:44:51Z

Looks like it hit the ValueError on the test_continue that was supposed to use a correct continue run. Is this a case where the action-tmate would help us debug?

            if continue_timesteps:
                completed_timesteps = operator.prev_res.get_times()
                completed_source_rates = operator.prev_res.get_source_rates()
                num_previous_steps_run = len(completed_timesteps)
                if (
                    np.array_equal(completed_timesteps, timesteps[: num_previous_steps_run]) and
                    np.array_equal(completed_source_rates, source_rates[: num_previous_steps_run])
                ):
                    seconds = seconds[num_previous_steps_run:]
                    source_rates = source_rates[num_previous_steps_run:]
                else:
>                   raise ValueError(
                        "You are attempting to continue a run in which the previous results "
                        "do not have the same initial steps as those provided to the "
                        "Integrator. Please make sure you are using the correct timesteps, "
                        "powers or power densities, and previous results file."
                    )
E                   ValueError: You are attempting to continue a run in which the previous results do not have the same initial steps as those provided to the Integrator. Please make sure you are using the correct timesteps, powers or power densities, and previous results file.

pshriwise

A few things to address here:

We'll want to make sure that the call to Results.get_times() provides the same unis as the time steps passed to the Integrator class here.
The times coming from the Results object are points in time, not dt, so we'll need to take the diff of that array to make the comparison to the time steps passed for the continuation run.
Similar to the get_times situation, the array coming from the Results.get_source_rates() will include more values than the number of time steps and will need to be modified to make a valid comparison to the continuation source rates.

pshriwise · 2025-01-29T22:52:09Z

Looks like it hit the ValueError on the test_continue that was supposed to use a correct continue run. Is this a case where the action-tmate would help us debug?

            if continue_timesteps:
                completed_timesteps = operator.prev_res.get_times()
                completed_source_rates = operator.prev_res.get_source_rates()
                num_previous_steps_run = len(completed_timesteps)
                if (
                    np.array_equal(completed_timesteps, timesteps[: num_previous_steps_run]) and
                    np.array_equal(completed_source_rates, source_rates[: num_previous_steps_run])
                ):
                    seconds = seconds[num_previous_steps_run:]
                    source_rates = source_rates[num_previous_steps_run:]
                else:
>                   raise ValueError(
                        "You are attempting to continue a run in which the previous results "
                        "do not have the same initial steps as those provided to the "
                        "Integrator. Please make sure you are using the correct timesteps, "
                        "powers or power densities, and previous results file."
                    )
E                   ValueError: You are attempting to continue a run in which the previous results do not have the same initial steps as those provided to the Integrator. Please make sure you are using the correct timesteps, powers or power densities, and previous results file.

Possibly yes, but this is an error that can be reproduced locally easily enough.

lewisgross1296 · 2025-02-04T22:55:25Z

Turn out some of the tests were triggering the ValueError for one of the two checks in the if statement. As suggested by @pshriwise, I broke them up into two blocks: one to check consistent timesteps and one to check consistent source rates.

Here's my local version of the code

            # validate existing depletion steps are consistent with those passed to operator
            if continue_timesteps:
                completed_times = operator.prev_res.get_times(time_units=timestep_units)
                completed_timesteps = completed_times[1:] - completed_times[:-1] # convert absolute t to dt
                completed_source_rates = operator.prev_res.get_source_rates()
                num_previous_steps_run = len(completed_timesteps)
                if (np.array_equal(completed_timesteps, timesteps[:num_previous_steps_run])):
                    seconds = seconds[num_previous_steps_run:]
                else:
                    print("timestep check failed")
                    print(f"timesteps given to continue run: {timesteps}")
                    print(f"found completed timesteps: {completed_timesteps}")
                    raise ValueError(
                        "You are attempting to continue a run in which the previous timesteps "
                        "do not have the same initial timesteps as those provided to the "
                        "Integrator. Please make sure you are using the correct timesteps."
                    )
                if(np.array_equal(completed_source_rates, source_rates[:num_previous_steps_run])):
                    source_rates = source_rates[num_previous_steps_run:]
                else:
                    print("source rate check failed")
                    print(f"SRs given to coninute run: {source_rates}")
                    print(f"completed SRs: {completed_source_rates}")
                    raise ValueError(
                        "You are attempting to continue a run in which the previous results "
                        "do not have the same initial source rates, powers, or power densities "
                        "as those provided to the Integrator. Please make sure you are using "
                        "the correct powers, power densities, or source rates and previous results file."
                    )

I've added some print statements to get an idea of what the important variables look like during tests but I'm not totally sure why some tests are failing despite printing values that seem to be the same?

test_deplete_continue.py::test_continue[predictor] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[cecm] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[celi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[cf4] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[epc_rk4] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[leqi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[si_leqi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[si_celi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
source rate check failed
SRs given to coninute run: [1.0, 1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_mismatched_initial_times[predictor] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 s, dt=0.75 s, source=1.0
[openmc.deplete] t=1.5 (final operator evaluation)
timestep check failed
timesteps given to continue run: [0.75, 0.5, 0.75]
found completed timesteps: [0.75 0.75]

I tested the numpy.array_equal function for 1. and 1.0

lgross@ulam:~/openmc (auto_continue_depletion) $ python
Python 3.11.11 (main, Jan 13 2025, 12:51:04) [GCC 13.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy
>>> print(numpy.array_equal([1.],[1.0]))
True

I know we discussed some schemes might have more data in them than the Integrator has but this doesn't seem to be happening in the above? Maybe I'm misinterpreting

lewisgross1296 · 2025-02-11T18:41:47Z

As discussed today, I printed the non-sliced versions of the data above which clears up why the arrays appeared the same. E.g. we should actually do this to test

                if(np.array_equal(completed_source_rates, source_rates[:num_previous_steps_run])):
                    source_rates = source_rates[num_previous_steps_run:]
                else:
                    print("source rate check failed")
                    print(f"Initial SRs given to coninute run: {source_rates[:num_previous_steps_run]}")
                    print(f"completed SRs: {completed_source_rates}")

When I print what's actually compared in the if statements, we see

test_deplete_continue.py::test_continue[predictor] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[cecm] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[celi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[cf4] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[epc_rk4] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[leqi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
[openmc.deplete] t=0.75 (final operator evaluation)
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[si_leqi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED
test_deplete_continue.py::test_continue[si_celi] [openmc.deplete] t=0.0 s, dt=0.75 s, source=1.0
source rate check failed
SRs (sliced) given to coninute run: [1.0]
completed SRs: [1. 1.]
FAILED

This is due to the fact that certain integrators with sub-steps write out more source rates than are initially provided. I will work on looking into how to capture this case and test appropriately.

lewisgross1296 · 2025-02-18T22:52:19Z

+    def get_source_rates(self) -> np.ndarray:
+        """
+        .. versionadded:: 0.15.1
+
+        Returns
+        -------
+        numpy.ndarray
+            1-D vector of source rates at each point in the depletion simulation
+            with the units originally defined by the user.
+
+        """
+        source_rates = np.fromiter(
+            (r.source_rate for r in self),
+            dtype=self[0].source_rate.dtype,
+            count=len(self),
+        )
+
+        return source_rates


It seems like when I print out the result of this function, it duplicates the final source rate (no matter the integrator chosen). When I change to count=len(self) -1, it prints what I would expect for the source_rates ndarray.

Thinking about this more, it kind of makes sense since get_times() returns an ndarray containing 0 as the first element. If there are N timesteps and N source rates given to the integrator, then in this case, count = len(self) -1 is the correct choice for this function.

Not sure if this is intended, but the final simulation "time" duplicates the system final source rate.

pshriwise

Looking good @lewisgross1296! Just a few line comments from me.

pshriwise · 2025-03-04T17:15:47Z

+_SECONDS_PER_MINUTE = 60
+_SECONDS_PER_HOUR = 60*60
+_SECONDS_PER_DAY = 24*60*60
+_SECONDS_PER_JULIAN_YEAR = 365.25*24*60*60


looks like we lost a comment from the original line here

pshriwise · 2025-03-04T17:17:18Z

+            (r.source_rate for r in self),
+            dtype=self[0].source_rate.dtype,
+            count=len(self)-1,
+        ) # Results duplicates the final source rate at the final simulation time


Let's move this comment above the code.

Suggested change

) # Results duplicates the final source rate at the final simulation time

) # Results duplicate the final source rate at the final simulation time

pshriwise · 2025-03-04T17:18:15Z

        .. versionadded:: 0.12
+    continue_timesteps : bool, optional
+        Whether or not to treat the current solve as a continuation of a
+        previous simulation. Defaults to `False`. If `True`, the timesteps


Let's explain the intended behavior for a False value as well here.

pshriwise · 2025-03-04T17:22:15Z

+        # retrieved from operator.prev_res.get_source_rates(), then run a depletion simulations
+        # with only the new time steps and source rates provided


Suggested change

# retrieved from operator.prev_res.get_source_rates(), then run a depletion simulations

# with only the new time steps and source rates provided

# retrieved from operator.prev_res.get_source_rates(), then run a depletion simulation

# with only the new time steps and source rates provided

Co-authored-by: Edgar-21 <84034227+Edgar-21@users.noreply.github.com> Co-authored-by: Connor Moreno <camoreno@wisc.edu>

…pdated doc strings

Co-authored-by: Patrick Shriwise <pshriwise@gmail.com> doc string and wrap match error message

Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>

lewisgross1296 · 2025-03-04T20:40:07Z

@connoramoreno @Edgar-21 @dean-krueger test are passing locally now, if you're good with the state of things, please 👍 and otherwise leave a review :)

pshriwise

Thanks @lewisgross1296! Nice to have this option to continue depletion runs without input updates!

paulromano

Thanks @lewisgross1296! I ended up making a few simplifications. The only notable change is that I don't think continue_timesteps=True should be allowed if no prev_results is provided (silently changing the value of continue_timesteps). If the user asks to continue the run, it stands to reason they should be providing previous results as well. Let me know if you're OK with that.

Co-authored-by: Connor Moreno <camoreno@wisc.edu> Co-authored-by: Patrick Shriwise <pshriwise@gmail.com> Co-authored-by: Lewis Gross <ligross@cnerg-docker-04.neep.wisc.edu> Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

lewisgross1296 requested a review from paulromano as a code owner January 22, 2025 03:20

lewisgross1296 commented Jan 22, 2025

View reviewed changes

Comment thread openmc/deplete/abc.py Outdated

lewisgross1296 force-pushed the auto_continue_depletion branch 2 times, most recently from 1b9dbbd to b5fe00b Compare January 22, 2025 22:20

dean-krueger reviewed Jan 24, 2025

View reviewed changes

Comment thread openmc/deplete/abc.py Outdated

Edgar-21 reviewed Jan 24, 2025

View reviewed changes

Comment thread openmc/deplete/abc.py Outdated

This comment was marked as outdated.

Sign in to view

connoramoreno reviewed Jan 24, 2025

View reviewed changes

Comment thread openmc/deplete/abc.py Outdated

pshriwise reviewed Jan 28, 2025

View reviewed changes

Comment thread tests/unit_tests/test_deplete_continue.py Outdated

pshriwise reviewed Jan 28, 2025

View reviewed changes

Comment thread tests/unit_tests/test_deplete_continue.py Outdated

pshriwise reviewed Jan 28, 2025

View reviewed changes

Comment thread tests/unit_tests/test_deplete_continue.py Outdated

lewisgross1296 mentioned this pull request Jan 28, 2025

No way to export source_rate units when writing step in depletion simulation #3281

Open

pshriwise requested changes Jan 29, 2025

View reviewed changes

lewisgross1296 commented Feb 18, 2025

View reviewed changes

pshriwise requested changes Mar 4, 2025

View reviewed changes

lewisgross1296 and others added 10 commits March 4, 2025 13:00

add continue feature for depletion and test new code

1730c21

Co-authored-by: Edgar-21 <84034227+Edgar-21@users.noreply.github.com> Co-authored-by: Connor Moreno <camoreno@wisc.edu>

added flags to SIIntegrator, convereted np.equal to np.array_equal, u…

5ed5a59

…pdated doc strings

implement get_source_rates

48f8478

split failure cases into two tests and added match param

136289c

source_rate does not have index (scalar variable)

46ab8b4

Co-authored-by: Patrick Shriwise <pshriwise@gmail.com> doc string and wrap match error message

fix off by 1

4fdf333

split checks, need to compute dts, need timestep_units in get_times call

3f96f1a

make sure that continue runs with no previous results do not break

f3358e4

fix indent issue, add some description

09df7c1

addressing some of Patrick's feedback

020aff2

lewisgross1296 force-pushed the auto_continue_depletion branch from 08f658f to 020aff2 Compare March 4, 2025 19:15

lewisgross1296 and others added 2 commits March 4, 2025 13:36

remove comment in favor of docstring

00e575d

imrove docstring

220f1a9

Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>

lewisgross1296 force-pushed the auto_continue_depletion branch from 7905cfc to 220f1a9 Compare March 4, 2025 20:13

Lewis Gross and others added 2 commits March 4, 2025 18:55

move completed_source_rates declaration after success of timestep check

1915e6d

Style changes to test file

384e99c

pshriwise approved these changes Mar 5, 2025

View reviewed changes

paulromano added 3 commits March 5, 2025 16:06

Revert change to pugixml submodule

471e5b6

Small stylistic fixes

b27c9df

Simplify tests for continue run

e2d0ad1

paulromano approved these changes Mar 5, 2025

View reviewed changes

lewisgross1296 commented Mar 5, 2025

View reviewed changes

Comment thread openmc/deplete/abc.py

paulromano enabled auto-merge (squash) March 6, 2025 00:58

paulromano merged commit 557b714 into openmc-dev:develop Mar 6, 2025

lewisgross1296 deleted the auto_continue_depletion branch March 6, 2025 01:14

lewisgross1296 mentioned this pull request Apr 25, 2025

restarting killed depletion simulations improperly saving h5 data #3387

Closed

lewisgross1296 mentioned this pull request May 19, 2025

Update _get_start_data to always grab the beginning of timestep time #3414

Merged

4 of 5 tasks

GuySten mentioned this pull request Aug 28, 2026

failure and error when restart depletion with MPI on #2878

Closed

	) # Results duplicates the final source rate at the final simulation time
	) # Results duplicate the final source rate at the final simulation time

		# retrieved from operator.prev_res.get_source_rates(), then run a depletion simulations
		# with only the new time steps and source rates provided

Conversation

lewisgross1296 commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

lewisgross1296 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dean-krueger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

connoramoreno left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewisgross1296 commented Jan 28, 2025

Uh oh!

pshriwise left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pshriwise commented Jan 29, 2025

Uh oh!

lewisgross1296 commented Feb 4, 2025

Uh oh!

lewisgross1296 commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lewisgross1296 Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

pshriwise left a comment

Choose a reason for hiding this comment

Uh oh!

pshriwise Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

pshriwise Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

pshriwise Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pshriwise Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewisgross1296 commented Mar 4, 2025

Uh oh!

pshriwise left a comment

Choose a reason for hiding this comment

Uh oh!

paulromano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

lewisgross1296 commented Jan 22, 2025 •

edited

Loading

lewisgross1296 left a comment •

edited

Loading

connoramoreno left a comment •

edited

Loading

lewisgross1296 commented Feb 11, 2025 •

edited

Loading