From 1891ff7c2420e91701e41c9f9556e2938170649e Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 13:47:42 -0600 Subject: [PATCH 01/12] Updating batch number checks for simulation and sp load --- src/simulation.cpp | 2 +- src/state_point.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/simulation.cpp b/src/simulation.cpp index 2f07f8124f..7740c81d80 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -236,7 +236,7 @@ int openmc_next_batch(int* status) // Check simulation ending criteria if (status) { - if (simulation::current_batch == settings::n_max_batches) { + if (simulation::current_batch >= settings::n_max_batches) { *status = STATUS_EXIT_MAX_BATCH; } else if (simulation::satisfy_triggers) { *status = STATUS_EXIT_ON_TRIGGER; diff --git a/src/state_point.cpp b/src/state_point.cpp index 4170421bec..8ca0166c63 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -403,9 +403,10 @@ void load_state_point() // Read batch number to restart at read_dataset(file_id, "current_batch", simulation::restart_batch); - if (simulation::restart_batch > settings::n_batches) { - fatal_error("The number batches specified in settings.xml is fewer " - " than the number of batches in the given statepoint file."); + if (simulation::restart_batch >= settings::n_batches) { + fatal_error(fmt::format("The number of batches specified for simiulation ({}) is smaller" + " than the number of batches in the restart statepoint file ({})", + settings::n_batches, simulation::restart_batch)); } // Logical flag for source present in statepoint file From 0e10a7da47ddbea8fd43236db59f3d414124c33b Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 13:48:11 -0600 Subject: [PATCH 02/12] Allowing openmc.Pathlike for restart file argument --- openmc/executor.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openmc/executor.py b/openmc/executor.py index accffd06ac..e779fdde4b 100644 --- a/openmc/executor.py +++ b/openmc/executor.py @@ -3,6 +3,7 @@ from numbers import Integral import subprocess import openmc +from openmc.checkvalue import PathLike from .plots import _get_plot_image @@ -73,8 +74,8 @@ def _process_CLI_arguments(volume=False, geometry_debug=False, particles=None, if event_based: args.append('-e') - if isinstance(restart_file, str): - args += ['-r', restart_file] + if isinstance(restart_file, PathLike.__args__): + args += ['-r', str(restart_file)] if tracks: args.append('-t') From ee2f0f2c53048bb7dafed8f270cad2668ea5da14 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 13:48:28 -0600 Subject: [PATCH 03/12] Adding a test for restart runs --- tests/unit_tests/test_restart.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 tests/unit_tests/test_restart.py diff --git a/tests/unit_tests/test_restart.py b/tests/unit_tests/test_restart.py new file mode 100644 index 0000000000..f2d99e0820 --- /dev/null +++ b/tests/unit_tests/test_restart.py @@ -0,0 +1,23 @@ +import openmc + +import pytest + +def test_restart(run_in_tmpdir): + + pincell = openmc.examples.pwr_pin_cell() + + # run the pincell + sp_file = pincell.run() + + # run a restart with the resulting statepoint + # and the settings unchanged + + with pytest.raises(RuntimeError, match='is smaller than the number of batches'): + pincell.run(restart_file=sp_file) + + # update the number of batches and run again + pincell.settings.batches = 15 + sp_file = pincell.run(restart_file=sp_file) + + sp = openmc.StatePoint(sp_file) + assert sp.n_batches == 15 \ No newline at end of file From 461fd54f11a74ce7713b8717661566905db76483 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 13:53:57 -0600 Subject: [PATCH 04/12] Updating doc strings --- openmc/executor.py | 10 +++++----- openmc/geometry.py | 2 +- openmc/model/model.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/openmc/executor.py b/openmc/executor.py index e779fdde4b..758604c91e 100644 --- a/openmc/executor.py +++ b/openmc/executor.py @@ -24,7 +24,7 @@ def _process_CLI_arguments(volume=False, geometry_debug=False, particles=None, Number of particles to simulate per generation. plot : bool, optional Run in plotting mode. Defaults to False. - restart_file : str, optional + restart_file : str or PathLike Path to restart file to use threads : int, optional Number of OpenMP threads. If OpenMC is compiled with OpenMP threading @@ -43,7 +43,7 @@ def _process_CLI_arguments(volume=False, geometry_debug=False, particles=None, mpi_args : list of str, optional MPI execute command and any additional MPI arguments to pass, e.g., ['mpiexec', '-n', '8']. - path_input : str or Pathlike + path_input : str or PathLike Path to a single XML file or a directory containing XML files for the OpenMC executable to read. @@ -231,7 +231,7 @@ def calculate_volumes(threads=None, output=True, cwd='.', cwd : str, optional Path to working directory to run in. Defaults to the current working directory. - path_input : str or Pathlike + path_input : str or PathLike Path to a single XML file or a directory containing XML files for the OpenMC executable to read. @@ -271,7 +271,7 @@ def run(particles=None, threads=None, geometry_debug=False, :envvar:`OMP_NUM_THREADS` environment variable). geometry_debug : bool, optional Turn on geometry debugging during simulation. Defaults to False. - restart_file : str, optional + restart_file : str or PathLike Path to restart file to use tracks : bool, optional Enables the writing of particles tracks. The number of particle tracks @@ -292,7 +292,7 @@ def run(particles=None, threads=None, geometry_debug=False, .. versionadded:: 0.12 - path_input : str or Pathlike + path_input : str or PathLike Path to a single XML file or a directory containing XML files for the OpenMC executable to read. diff --git a/openmc/geometry.py b/openmc/geometry.py index 511a3da409..d01826a4dd 100644 --- a/openmc/geometry.py +++ b/openmc/geometry.py @@ -278,7 +278,7 @@ class Geometry: """ - # Using str and os.Pathlike here to avoid error when using just the imported PathLike + # Using str and os.PathLike here to avoid error when using just the imported PathLike # TypeError: Subscripted generics cannot be used with class and instance checks check_type('materials', materials, (str, os.PathLike, openmc.Materials)) diff --git a/openmc/model/model.py b/openmc/model/model.py index 6ce6ef33ae..79708d62b3 100644 --- a/openmc/model/model.py +++ b/openmc/model/model.py @@ -248,7 +248,7 @@ class Model: Parameters ---------- - path : str or Pathlike + path : str or PathLike Path to model.xml file """ tree = ET.parse(path) @@ -473,7 +473,7 @@ class Model: Parameters ---------- - path : str or Pathlike + path : str or PathLike Location of the XML file to write (default is 'model.xml'). Can be a directory or file path. remove_surfs : bool @@ -620,7 +620,7 @@ class Model: value set by the :envvar:`OMP_NUM_THREADS` environment variable). geometry_debug : bool, optional Turn on geometry debugging during simulation. Defaults to False. - restart_file : str, optional + restart_file : str or PathLike Path to restart file to use tracks : bool, optional Enables the writing of particles tracks. The number of particle From 9803c2611196bb9d9e89ef46e24ab8c2d4b7e17d Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 16:18:41 -0600 Subject: [PATCH 05/12] bwd compatibility fix for reading filter elements --- openmc/filter.py | 2 ++ tests/unit_tests/test_restart.py | 23 ----------------------- 2 files changed, 2 insertions(+), 23 deletions(-) delete mode 100644 tests/unit_tests/test_restart.py diff --git a/openmc/filter.py b/openmc/filter.py index c52d3f135e..1f2f9816ca 100644 --- a/openmc/filter.py +++ b/openmc/filter.py @@ -258,6 +258,8 @@ class Filter(IDManagerMixin, metaclass=FilterMeta): """ filter_type = elem.get('type') + if filter_type is None: + filter_type = elem.find('type').text # If the filter type matches this class's short_name, then # there is no overridden from_xml_element method diff --git a/tests/unit_tests/test_restart.py b/tests/unit_tests/test_restart.py deleted file mode 100644 index f2d99e0820..0000000000 --- a/tests/unit_tests/test_restart.py +++ /dev/null @@ -1,23 +0,0 @@ -import openmc - -import pytest - -def test_restart(run_in_tmpdir): - - pincell = openmc.examples.pwr_pin_cell() - - # run the pincell - sp_file = pincell.run() - - # run a restart with the resulting statepoint - # and the settings unchanged - - with pytest.raises(RuntimeError, match='is smaller than the number of batches'): - pincell.run(restart_file=sp_file) - - # update the number of batches and run again - pincell.settings.batches = 15 - sp_file = pincell.run(restart_file=sp_file) - - sp = openmc.StatePoint(sp_file) - assert sp.n_batches == 15 \ No newline at end of file From 648313fab16719d526adb69444bf18303e94ec0d Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 16:28:05 -0600 Subject: [PATCH 06/12] Adding to regression test instead of making new unit test --- .../statepoint_restart/test.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/regression_tests/statepoint_restart/test.py b/tests/regression_tests/statepoint_restart/test.py index 4575607f7d..9ab528e617 100644 --- a/tests/regression_tests/statepoint_restart/test.py +++ b/tests/regression_tests/statepoint_restart/test.py @@ -1,11 +1,14 @@ import glob import os +from pathlib import Path import openmc from tests.testing_harness import TestHarness from tests.regression_tests import config +from tests import cdtemp +import pytest class StatepointRestartTestHarness(TestHarness): def __init__(self, final_sp, restart_sp): @@ -59,3 +62,27 @@ def test_statepoint_restart(): harness = StatepointRestartTestHarness('statepoint.10.h5', 'statepoint.07.h5') harness.main() + + +def test_batch_check(request): + xmls = glob.glob('*.xml') + xmls = [request.fspath.dirpath() / Path(f) for f in xmls] + + with cdtemp(xmls): + model = openmc.Model.from_xml() + model.settings.particles = 100 + # run the model + sp_file = model.run() + + # run a restart with the resulting statepoint + # and the settings unchanged + with pytest.raises(RuntimeError, match='is smaller than the number of batches'): + model.run(restart_file=sp_file) + + # update the number of batches and run again + model.settings.batches = 15 + model.settings.statepoint = {} + sp_file = model.run(restart_file=sp_file) + + sp = openmc.StatePoint(sp_file) + assert sp.n_batches == 15 \ No newline at end of file From 65852f4fbc8e2cbe702af558733aff0bae83d1a3 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 16 Feb 2023 17:52:23 -0600 Subject: [PATCH 07/12] Update batch comparison in statepoint load --- src/state_point.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/state_point.cpp b/src/state_point.cpp index 8ca0166c63..23538da59d 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -403,10 +403,11 @@ void load_state_point() // Read batch number to restart at read_dataset(file_id, "current_batch", simulation::restart_batch); - if (simulation::restart_batch >= settings::n_batches) { - fatal_error(fmt::format("The number of batches specified for simiulation ({}) is smaller" - " than the number of batches in the restart statepoint file ({})", - settings::n_batches, simulation::restart_batch)); + if (simulation::restart_batch >= settings::n_max_batches) { + fatal_error(fmt::format( + "The number of batches specified for simiulation ({}) is smaller" + " than the number of batches in the restart statepoint file ({})", + settings::n_max_batches, simulation::restart_batch)); } // Logical flag for source present in statepoint file From b8c7c7644422a7bd8b83cb096c3b50c4ef97ce00 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 21 Feb 2023 00:42:27 -0600 Subject: [PATCH 08/12] Apply suggestions from @paulromano Co-authored-by: Paul Romano --- src/state_point.cpp | 2 +- tests/regression_tests/statepoint_restart/test.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/state_point.cpp b/src/state_point.cpp index 23538da59d..dfd3d381b5 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -405,7 +405,7 @@ void load_state_point() if (simulation::restart_batch >= settings::n_max_batches) { fatal_error(fmt::format( - "The number of batches specified for simiulation ({}) is smaller" + "The number of batches specified for simulation ({}) is smaller" " than the number of batches in the restart statepoint file ({})", settings::n_max_batches, simulation::restart_batch)); } diff --git a/tests/regression_tests/statepoint_restart/test.py b/tests/regression_tests/statepoint_restart/test.py index 9ab528e617..35df4f07d2 100644 --- a/tests/regression_tests/statepoint_restart/test.py +++ b/tests/regression_tests/statepoint_restart/test.py @@ -3,13 +3,12 @@ import os from pathlib import Path import openmc +import pytest from tests.testing_harness import TestHarness from tests.regression_tests import config from tests import cdtemp -import pytest - class StatepointRestartTestHarness(TestHarness): def __init__(self, final_sp, restart_sp): super().__init__(final_sp) @@ -65,8 +64,7 @@ def test_statepoint_restart(): def test_batch_check(request): - xmls = glob.glob('*.xml') - xmls = [request.fspath.dirpath() / Path(f) for f in xmls] + xmls = list(request.path.parent.glob('*.xml')) with cdtemp(xmls): model = openmc.Model.from_xml() From 1b68e61a3edd3290d587c2dcb9d752523599c452 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 21 Feb 2023 00:47:47 -0600 Subject: [PATCH 09/12] explicit instance check --- openmc/executor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openmc/executor.py b/openmc/executor.py index 758604c91e..aacc48b3fa 100644 --- a/openmc/executor.py +++ b/openmc/executor.py @@ -1,9 +1,9 @@ from collections.abc import Iterable from numbers import Integral +import os import subprocess import openmc -from openmc.checkvalue import PathLike from .plots import _get_plot_image @@ -74,7 +74,7 @@ def _process_CLI_arguments(volume=False, geometry_debug=False, particles=None, if event_based: args.append('-e') - if isinstance(restart_file, PathLike.__args__): + if isinstance(restart_file, (str, os.PathLike)): args += ['-r', str(restart_file)] if tracks: From 0aa36836b3a73818fb9d29b4a8ac13150dd6df5b Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 21 Feb 2023 00:48:23 -0600 Subject: [PATCH 10/12] Updating globs --- tests/regression_tests/statepoint_restart/test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/regression_tests/statepoint_restart/test.py b/tests/regression_tests/statepoint_restart/test.py index 35df4f07d2..2f3a6ce884 100644 --- a/tests/regression_tests/statepoint_restart/test.py +++ b/tests/regression_tests/statepoint_restart/test.py @@ -1,4 +1,3 @@ -import glob import os from pathlib import Path @@ -44,7 +43,7 @@ class StatepointRestartTestHarness(TestHarness): def _run_openmc_restart(self): # Get the name of the statepoint file. - statepoint = glob.glob(os.path.join(os.getcwd(), self._restart_sp)) + statepoint = list(Path(os.getcwd()).glob(self._restart_sp)) assert len(statepoint) == 1 statepoint = statepoint[0] @@ -83,4 +82,4 @@ def test_batch_check(request): sp_file = model.run(restart_file=sp_file) sp = openmc.StatePoint(sp_file) - assert sp.n_batches == 15 \ No newline at end of file + assert sp.n_batches == 15 From 0419367815a3f552ccb1b994f7da2d8ecffc15d3 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 21 Feb 2023 07:28:32 -0600 Subject: [PATCH 11/12] Update tests/regression_tests/statepoint_restart/test.py Co-authored-by: Paul Romano --- tests/regression_tests/statepoint_restart/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/regression_tests/statepoint_restart/test.py b/tests/regression_tests/statepoint_restart/test.py index 2f3a6ce884..8acd0e88b6 100644 --- a/tests/regression_tests/statepoint_restart/test.py +++ b/tests/regression_tests/statepoint_restart/test.py @@ -43,7 +43,7 @@ class StatepointRestartTestHarness(TestHarness): def _run_openmc_restart(self): # Get the name of the statepoint file. - statepoint = list(Path(os.getcwd()).glob(self._restart_sp)) + statepoint = list(Path.cwd().glob(self._restart_sp)) assert len(statepoint) == 1 statepoint = statepoint[0] From 55179b5a46accdf8b8dfcd8ce1ed3c0f454a92f4 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 21 Feb 2023 07:31:53 -0600 Subject: [PATCH 12/12] Removing os module from restart test --- tests/regression_tests/statepoint_restart/test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/regression_tests/statepoint_restart/test.py b/tests/regression_tests/statepoint_restart/test.py index 8acd0e88b6..1e98bc480b 100644 --- a/tests/regression_tests/statepoint_restart/test.py +++ b/tests/regression_tests/statepoint_restart/test.py @@ -1,4 +1,3 @@ -import os from pathlib import Path import openmc