From c9e91d42fbc0a8aca2e7cd70f464ee940c848a08 Mon Sep 17 00:00:00 2001 From: agnelson Date: Tue, 5 Oct 2021 08:39:24 -0500 Subject: [PATCH] Replacing manual absolute error tests with pytest.approx, improving documentation, and other small changes per the review by @paulroman. --- openmc/executor.py | 2 +- openmc/lib/core.py | 20 +++++++++---- openmc/model/model.py | 12 ++++---- tests/unit_tests/test_model.py | 53 +++++++++++++++++++--------------- 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/openmc/executor.py b/openmc/executor.py index c2974722c..2d4911ff2 100644 --- a/openmc/executor.py +++ b/openmc/executor.py @@ -52,7 +52,7 @@ def _process_CLI_arguments(volume=False, geometry_debug=False, particles=None, args = [openmc_exec] if volume: - args += ['--volume'] + args.append('--volume') if isinstance(particles, Integral) and particles > 0: args += ['-n', str(particles)] diff --git a/openmc/lib/core.py b/openmc/lib/core.py index 136e5a6d8..e277d527a 100644 --- a/openmc/lib/core.py +++ b/openmc/lib/core.py @@ -112,7 +112,17 @@ def global_bounding_box(): def calculate_volumes(output=True): - """Run stochastic volume calculation""" + """Run stochastic volume calculation + + .. versionchanged:: 0.13.0 + The *output* argument was added. + + Parameters + ---------- + output : bool, optional + Whether or not to show output. Defaults to showing output + + """ with quiet_dll(output): _dll.openmc_calculate_volumes() @@ -140,7 +150,7 @@ def export_properties(filename=None, output=True): ---------- filename : str or None Filename to export properties to (defaults to "properties.h5") - output: bool, optional + output : bool, optional Whether or not to show output. Defaults to showing output See Also @@ -242,7 +252,7 @@ def init(args=None, intracomm=None, output=True): Command-line arguments intracomm : mpi4py.MPI.Intracomm or None, optional MPI intracommunicator - output: bool, optional + output : bool, optional Whether or not to show output. Defaults to showing output """ @@ -369,7 +379,7 @@ def plot_geometry(output=True): Parameters ---------- - output: bool, optional + output : bool, optional Whether or not to show output. Defaults to showing output """ @@ -395,7 +405,7 @@ def run(output=True): Parameters ---------- - output: bool, optional + output : bool, optional Whether or not to show output. Defaults to showing output """ diff --git a/openmc/model/model.py b/openmc/model/model.py index 904c60746..ad898fd0a 100644 --- a/openmc/model/model.py +++ b/openmc/model/model.py @@ -20,7 +20,7 @@ from openmc.exceptions import InvalidIDError @contextmanager def _change_directory(working_dir): """A context manager for executing in a provided working directory""" - start_dir = Path().cwd() + start_dir = Path.cwd() Path.mkdir(working_dir, exist_ok=True) os.chdir(working_dir) try: @@ -231,7 +231,7 @@ class Model: event_based : None or bool, optional Turns on event-based parallelism if True. If None, the value in the Settings will be used. - intracomm : DummyCommnicator, mpi4py.MPI.Intracomm or None, optional + intracomm : mpi4py.MPI.Intracomm or None, optional MPI intracommunicator """ @@ -261,7 +261,10 @@ class Model: self.export_to_xml() self._intracomm.barrier() - openmc.lib.init(args=args, intracomm=self._intracomm, output=output) + # We cannot pass DummyCommunicator to openmc.lib.init so pass instead + # the user-provided intracomm which will either be None or an mpi4py + # communicator + openmc.lib.init(args=args, intracomm=intracomm, output=output) def finalize_lib(self): """Finalize simulation and free memory allocated for the C API @@ -610,8 +613,7 @@ class Model: if apply_volumes: # Load the results and add them to the model for i, vol_calc in enumerate(self.settings.volume_calculations): - f_name = f"volume_{i + 1}.h5" - vol_calc.load_results(f_name) + vol_calc.load_results(f"volume_{i + 1}.h5") # First add them to the Python side self.geometry.add_volume_information(vol_calc) diff --git a/tests/unit_tests/test_model.py b/tests/unit_tests/test_model.py index cffa5c249..b1021a269 100644 --- a/tests/unit_tests/test_model.py +++ b/tests/unit_tests/test_model.py @@ -1,8 +1,10 @@ from math import pi -import numpy as np -import pytest from pathlib import Path from shutil import which + +import numpy as np +import pytest + import openmc import openmc.lib @@ -304,9 +306,9 @@ def test_run(run_in_tmpdir, pin_model_attributes, mpi_intracomm): lib_fiss = sp.get_tally(id=1).get_values(scores=['fission'])[0, 0, 0] # and lets compare results - assert abs(lib_keff - cli_keff) < 1e-13 - assert abs(lib_flux - cli_flux) < 1e-13 - assert abs(lib_fiss - cli_fiss) < 1e-13 + assert lib_keff.n == pytest.approx(cli_keff.n, abs=1e-13) + assert lib_flux == pytest.approx(cli_flux, abs=1e-13) + assert lib_fiss == pytest.approx(cli_fiss, abs=1e-13) # Now we should make sure that the flags for items which should be handled # by init are properly set @@ -348,7 +350,7 @@ def test_plots(run_in_tmpdir, pin_model_attributes, mpi_intracomm): # convert is True, test.png, plot_2.png for fname in ['test.', 'plot_2.']: for ext in exts: - test_file = Path('./{}{}'.format(fname, ext)) + test_file = Path(f'./{fname}{ext}') assert test_file.exists() test_file.unlink() @@ -398,19 +400,20 @@ def test_py_lib_attributes(run_in_tmpdir, pin_model_attributes, mpi_intracomm): # Now lets do the density updates. # Check initial conditions - assert abs(openmc.lib.materials[1].get_density( - 'atom/b-cm') - 0.06891296988603757) < 1e-13 + assert openmc.lib.materials[1].get_density('atom/b-cm') == \ + pytest.approx(0.06891296988603757, abs=1e-13) mat_a_dens = np.sum( [v[1] for v in test_model.materials[0]. get_nuclide_atom_densities().values()]) - assert abs(mat_a_dens - 0.06891296988603757) < 1e-8 + assert mat_a_dens == pytest.approx(0.06891296988603757, abs=1e-8) # Change the density test_model.update_densities(['UO2'], 2.) - assert abs(openmc.lib.materials[1].get_density('atom/b-cm') - 2.) < 1e-13 + assert openmc.lib.materials[1].get_density('atom/b-cm') == \ + pytest.approx(2., abs=1e-13) mat_a_dens = np.sum( [v[1] for v in test_model.materials[0]. get_nuclide_atom_densities().values()]) - assert abs(mat_a_dens - 2.) < 1e-8 + assert mat_a_dens == pytest.approx(2., abs=1e-8) # Now lets do the cell temperature updates. # Check initial conditions @@ -418,22 +421,26 @@ def test_py_lib_attributes(run_in_tmpdir, pin_model_attributes, mpi_intracomm): {2: geom.root_universe.cells[2], 3: geom.root_universe.cells[3], 4: geom.root_universe.cells[4], 1: geom.root_universe.cells[2].fill.cells[1]} - assert abs(openmc.lib.cells[3].get_temperature() - 293.6) < 1e-13 + assert openmc.lib.cells[3].get_temperature() == \ + pytest.approx(293.6, abs=1e-13) assert test_model.geometry.root_universe.cells[3].temperature is None # Change the temperature test_model.update_cell_temperatures([3], 600.) - assert abs(openmc.lib.cells[3].get_temperature() - 600.) < 1e-13 - assert abs(test_model.geometry.root_universe.cells[3].temperature - - 600.) < 1e-13 + assert openmc.lib.cells[3].get_temperature() == \ + pytest.approx(600., abs=1e-13) + assert test_model.geometry.root_universe.cells[3].temperature == \ + pytest.approx(600., abs=1e-13) # And finally material volume - assert abs(openmc.lib.materials[1].volume - 0.4831931368640985) < 1e-13 + assert openmc.lib.materials[1].volume == \ + pytest.approx(0.4831931368640985, abs=1e-13) # The temperature on the material will be None because its just the default - assert abs(test_model.materials[0].volume - 0.4831931368640985) < 1e-13 + assert test_model.materials[0].volume == \ + pytest.approx(0.4831931368640985, abs=1e-13) # Change the temperature test_model.update_material_volumes(['UO2'], 2.) - assert abs(openmc.lib.materials[1].volume - 2.) < 1e-13 - assert abs(test_model.materials[0].volume - 2.) < 1e-13 + assert openmc.lib.materials[1].volume == pytest.approx(2., abs=1e-13) + assert test_model.materials[0].volume == pytest.approx(2., abs=1e-13) test_model.finalize_lib() @@ -460,7 +467,7 @@ def test_deplete(run_in_tmpdir, pin_model_attributes, mpi_intracomm): # Get the new Xe136 and U235 atom densities after_xe = mats[0].get_nuclide_atom_densities()['Xe136'][1] after_u = mats[0].get_nuclide_atom_densities()['U235'][1] - assert abs((after_xe + after_u) - initial_u) < 1e-15 + assert after_xe + after_u == pytest.approx(initial_u, abs=1e-15) assert test_model.is_initialized is False # Reset the initial material densities @@ -480,12 +487,12 @@ def test_deplete(run_in_tmpdir, pin_model_attributes, mpi_intracomm): # Get the new Xe136 and U235 atom densities after_lib_xe = mats[0].get_nuclide_atom_densities()['Xe136'][1] after_lib_u = mats[0].get_nuclide_atom_densities()['U235'][1] - assert abs((after_lib_xe + after_lib_u) - initial_u) < 1e-15 + assert after_lib_xe + after_lib_u == pytest.approx(initial_u, abs=1e-15) assert test_model.is_initialized is True # And end by comparing to the previous case - assert abs(after_xe - after_lib_xe) < 1e-15 - assert abs(after_u - after_lib_u) < 1e-15 + assert after_xe == pytest.approx(after_lib_xe, abs=1e-15) + assert after_u == pytest.approx(after_lib_u, abs=1e-15) test_model.finalize_lib()