From abd1aa658643330cf0ac77f998a8ee5cf7d7d885 Mon Sep 17 00:00:00 2001 From: GuySten Date: Sat, 29 Aug 2026 02:00:17 +0300 Subject: [PATCH 1/4] fix --- openmc/deplete/keff_search_control.py | 9 + .../test_deplete_keff_search_control.py | 178 +++++++----------- 2 files changed, 74 insertions(+), 113 deletions(-) mode change 100644 => 100755 tests/unit_tests/test_deplete_keff_search_control.py diff --git a/openmc/deplete/keff_search_control.py b/openmc/deplete/keff_search_control.py index 49f7cc4dff3..d9465a8a21a 100644 --- a/openmc/deplete/keff_search_control.py +++ b/openmc/deplete/keff_search_control.py @@ -54,6 +54,15 @@ def run(self, x): root : float Parameter value that achieves target keff """ + # The keff search happens before the transport operator is called for + # this step, so both openmc.lib.materials and the operator's AtomNumber + # still hold the compositions from the previous operator call. Push the + # current beginning-of-step compositions in first, otherwise the search + # is performed on stale materials and _update_vec() below overwrites + # `x` with those stale densities, freezing the composition at its + # initial state for the entire depletion calculation. + self.operator._update_materials_and_nuclides(x) + root = self._search_for_keff() self._update_vec(x) return root diff --git a/tests/unit_tests/test_deplete_keff_search_control.py b/tests/unit_tests/test_deplete_keff_search_control.py old mode 100644 new mode 100755 index 425b8f84053..ae9cfe4cfbe --- a/tests/unit_tests/test_deplete_keff_search_control.py +++ b/tests/unit_tests/test_deplete_keff_search_control.py @@ -1,116 +1,68 @@ -""" Tests for KeffSearchControl class """ +"""Unit tests for openmc.deplete.keff_search_control.""" -from pathlib import Path - -import pytest import numpy as np +import pytest + +from openmc.deplete.keff_search_control import _KeffSearchControl + + +class MockOperator: + """Minimal operator recording calls to _update_materials_and_nuclides.""" + + def __init__(self, calls): + self.calls = calls + + def _update_materials_and_nuclides(self, vec): + self.calls.append(('update_materials', [v.copy() for v in vec])) + + +@pytest.fixture +def control_and_calls(monkeypatch): + calls = [] + operator = MockOperator(calls) + control = _KeffSearchControl( + operator, lambda x: None, x0=0.0, x1=1.0, bracket=[0.0, 2.0]) + + def fake_search(): + calls.append(('search', None)) + return 0.5 + + def fake_update_vec(x): + calls.append(('update_vec', None)) + + monkeypatch.setattr(control, '_search_for_keff', fake_search) + monkeypatch.setattr(control, '_update_vec', fake_update_vec) + return control, calls + + +def test_materials_updated_before_search(control_and_calls): + """Compositions must be pushed to openmc.lib before the search runs. + + The keff search is executed at the beginning of a depletion step, before + the transport operator is called. Without an explicit update, both + openmc.lib.materials and the operator's AtomNumber still hold the previous + call's compositions, and _update_vec() overwrites the depleted vector with + them -- freezing nuclide densities at their initial values. + """ + control, calls = control_and_calls + + n = [np.array([1.0, 2.0, 3.0]), np.array([4.0, 5.0, 6.0])] + root = control.run(n) + + assert root == 0.5 + assert [name for name, _ in calls] == [ + 'update_materials', 'search', 'update_vec'] + + # The vector handed to the operator must be the current composition + recorded = calls[0][1] + assert len(recorded) == len(n) + for actual, expected in zip(recorded, n): + np.testing.assert_array_equal(actual, expected) + -import openmc -import openmc.lib -from openmc.deplete import CoupledOperator - -CHAIN_PATH = Path(__file__).parents[1] / "chain_simple.xml" - - -def make_model(): - f = openmc.Material(name="fuel") - f.add_element("U", 1, percent_type="ao", enrichment=4.25) - f.add_element("O", 2) - f.set_density("g/cc", 10.4) - f.temperature = 293.15 - - w = openmc.Material(name="water") - w.add_element("O", 1) - w.add_element("H", 2) - w.set_density("g/cc", 1.0) - w.temperature = 293.15 - w.depletable = True - - h = openmc.Material(name='helium') - h.add_element('He', 1) - h.set_density('g/cm3', 0.001598) - - radii = [0.42, 0.45] - height = 0.5 - - f.volume = np.pi * radii[0] ** 2 * height - w.volume = np.pi * (radii[1]**2 - radii[0]**2) * height/2 - - materials = openmc.Materials([f, w, h]) - - surf_interface = openmc.ZPlane(z0=0) - surf_top = openmc.ZPlane(z0=height/2) - surf_bot = openmc.ZPlane(z0=-height/2) - surf_in = openmc.Sphere(r=radii[0]) - surf_out = openmc.Sphere(r=radii[1], boundary_type='vacuum') - - cell_water = openmc.Cell(fill=w, region=-surf_interface) - cell_helium = openmc.Cell(fill=h, region=+surf_interface) - universe = openmc.Universe(cells=(cell_water, cell_helium)) - cell_fuel = openmc.Cell(name='fuel_cell', fill=f, - region=-surf_in & -surf_top & +surf_bot) - cell_universe = openmc.Cell(name='universe_cell',fill=universe, - region=+surf_in & -surf_out & -surf_top & +surf_bot) - geometry = openmc.Geometry([cell_fuel, cell_universe]) - - settings = openmc.Settings() - settings.particles = 1000 - settings.inactive = 10 - settings.batches = 50 - - return openmc.Model(geometry, materials, settings) - - -def translate_cell(position): - """Helper function to translate a cell""" - cell = [c for c in openmc.lib.cells.values() if c.name == 'universe_cell'][0] - openmc.lib.cells[cell.id].translation = [0, 0, position] - return position - - -def rotate_cell(angle): - """Helper function to rotate a cell""" - cell = [c for c in openmc.lib.cells.values() if c.name == 'universe_cell'][0] - openmc.lib.cells[cell.id].rotation = [0, 0, angle] - return angle - - -def set_u235_density(u235_density): - """Helper function to set the U235 density directly""" - fuel = [m for m in openmc.lib.materials.values() if m.name == 'fuel'][0] - nuclides = openmc.lib.materials[fuel.id].nuclides - densities = openmc.lib.materials[fuel.id].densities - u235_idx = nuclides.index('U235') - densities[u235_idx] = u235_density - openmc.lib.materials[fuel.id].set_densities(nuclides, densities) - return u235_density - - -@pytest.mark.parametrize("function, x0, x1, bracket", [ - (translate_cell, -1.0, 1.0, (-5.0, 5.0)), - (rotate_cell, -45.0, 45.0, (-90.0, 90.0)), - (set_u235_density, 0.8, 1.2, (0.5, 1.5)) -]) -def test_integrator_add_keff_search_control(run_in_tmpdir, function, x0, x1, bracket): - """Test adding add_keff_search_control to integrator""" - model = make_model() - operator = CoupledOperator(model, CHAIN_PATH) - integrator = openmc.deplete.PredictorIntegrator( - operator, [1, 1], 0.0, timestep_units='d') - - integrator.add_keff_search_control( - function=function, - x0=x0, - x1=x1, - bracket=bracket, - k_tol=0.1, - output=False, - ) - - assert integrator._keff_search_control.x0 == x0 - assert integrator._keff_search_control.x1 == x1 - assert integrator._keff_search_control.function == function - assert integrator._keff_search_control.search_kwargs['x_min'] == bracket[0] - assert integrator._keff_search_control.search_kwargs['x_max'] == bracket[1] - assert integrator._keff_search_control.search_kwargs['k_tol'] == 0.1 - assert not integrator._keff_search_control.search_kwargs['output'] +def test_bracket_validation(): + operator = MockOperator([]) + with pytest.raises(ValueError, match='exactly 2 elements'): + _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [0.0]) + with pytest.raises(ValueError, match=r'bracket\[0\] must be'): + _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [2.0, 1.0]) From 1fdffa22e4aefe82fef7d51ad3132c6f09e650da Mon Sep 17 00:00:00 2001 From: GuySten Date: Sat, 29 Aug 2026 20:52:42 +0300 Subject: [PATCH 2/4] fix issue --- .../test_deplete_keff_search_control.py | 147 ++++++++++++++++-- 1 file changed, 130 insertions(+), 17 deletions(-) diff --git a/tests/unit_tests/test_deplete_keff_search_control.py b/tests/unit_tests/test_deplete_keff_search_control.py index ae9cfe4cfbe..a348e1b190f 100755 --- a/tests/unit_tests/test_deplete_keff_search_control.py +++ b/tests/unit_tests/test_deplete_keff_search_control.py @@ -1,43 +1,154 @@ -"""Unit tests for openmc.deplete.keff_search_control.""" +"""Tests for the KeffSearchControl class and openmc.deplete.keff_search_control.""" + +from pathlib import Path -import numpy as np import pytest +import numpy as np +import openmc +import openmc.lib +from openmc.deplete import CoupledOperator from openmc.deplete.keff_search_control import _KeffSearchControl +CHAIN_PATH = Path(__file__).parents[1] / "chain_simple.xml" + + +def make_model(): + f = openmc.Material(name="fuel") + f.add_element("U", 1, percent_type="ao", enrichment=4.25) + f.add_element("O", 2) + f.set_density("g/cc", 10.4) + f.temperature = 293.15 + + w = openmc.Material(name="water") + w.add_element("O", 1) + w.add_element("H", 2) + w.set_density("g/cc", 1.0) + w.temperature = 293.15 + w.depletable = True + + h = openmc.Material(name='helium') + h.add_element('He', 1) + h.set_density('g/cm3', 0.001598) + + radii = [0.42, 0.45] + height = 0.5 + + f.volume = np.pi * radii[0] ** 2 * height + w.volume = np.pi * (radii[1]**2 - radii[0]**2) * height/2 + + materials = openmc.Materials([f, w, h]) + + surf_interface = openmc.ZPlane(z0=0) + surf_top = openmc.ZPlane(z0=height/2) + surf_bot = openmc.ZPlane(z0=-height/2) + surf_in = openmc.Sphere(r=radii[0]) + surf_out = openmc.Sphere(r=radii[1], boundary_type='vacuum') + + cell_water = openmc.Cell(fill=w, region=-surf_interface) + cell_helium = openmc.Cell(fill=h, region=+surf_interface) + universe = openmc.Universe(cells=(cell_water, cell_helium)) + cell_fuel = openmc.Cell(name='fuel_cell', fill=f, + region=-surf_in & -surf_top & +surf_bot) + cell_universe = openmc.Cell(name='universe_cell',fill=universe, + region=+surf_in & -surf_out & -surf_top & +surf_bot) + geometry = openmc.Geometry([cell_fuel, cell_universe]) + + settings = openmc.Settings() + settings.particles = 1000 + settings.inactive = 10 + settings.batches = 50 + + return openmc.Model(geometry, materials, settings) + + +def translate_cell(position): + """Helper function to translate a cell""" + cell = [c for c in openmc.lib.cells.values() if c.name == 'universe_cell'][0] + openmc.lib.cells[cell.id].translation = [0, 0, position] + return position + + +def rotate_cell(angle): + """Helper function to rotate a cell""" + cell = [c for c in openmc.lib.cells.values() if c.name == 'universe_cell'][0] + openmc.lib.cells[cell.id].rotation = [0, 0, angle] + return angle + + +def set_u235_density(u235_density): + """Helper function to set the U235 density directly""" + fuel = [m for m in openmc.lib.materials.values() if m.name == 'fuel'][0] + nuclides = openmc.lib.materials[fuel.id].nuclides + densities = openmc.lib.materials[fuel.id].densities + u235_idx = nuclides.index('U235') + densities[u235_idx] = u235_density + openmc.lib.materials[fuel.id].set_densities(nuclides, densities) + return u235_density + class MockOperator: """Minimal operator recording calls to _update_materials_and_nuclides.""" - + def __init__(self, calls): self.calls = calls - + def _update_materials_and_nuclides(self, vec): self.calls.append(('update_materials', [v.copy() for v in vec])) - - + + @pytest.fixture def control_and_calls(monkeypatch): calls = [] operator = MockOperator(calls) control = _KeffSearchControl( operator, lambda x: None, x0=0.0, x1=1.0, bracket=[0.0, 2.0]) - + def fake_search(): calls.append(('search', None)) return 0.5 - + def fake_update_vec(x): calls.append(('update_vec', None)) - + monkeypatch.setattr(control, '_search_for_keff', fake_search) monkeypatch.setattr(control, '_update_vec', fake_update_vec) return control, calls - - + + +@pytest.mark.parametrize("function, x0, x1, bracket", [ + (translate_cell, -1.0, 1.0, (-5.0, 5.0)), + (rotate_cell, -45.0, 45.0, (-90.0, 90.0)), + (set_u235_density, 0.8, 1.2, (0.5, 1.5)) +]) +def test_integrator_add_keff_search_control(run_in_tmpdir, function, x0, x1, bracket): + """Test adding add_keff_search_control to integrator""" + model = make_model() + operator = CoupledOperator(model, CHAIN_PATH) + integrator = openmc.deplete.PredictorIntegrator( + operator, [1, 1], 0.0, timestep_units='d') + + integrator.add_keff_search_control( + function=function, + x0=x0, + x1=x1, + bracket=bracket, + k_tol=0.1, + output=False, + ) + + assert integrator._keff_search_control.x0 == x0 + assert integrator._keff_search_control.x1 == x1 + assert integrator._keff_search_control.function == function + assert integrator._keff_search_control.search_kwargs['x_min'] == bracket[0] + assert integrator._keff_search_control.search_kwargs['x_max'] == bracket[1] + assert integrator._keff_search_control.search_kwargs['k_tol'] == 0.1 + assert not integrator._keff_search_control.search_kwargs['output'] + + def test_materials_updated_before_search(control_and_calls): """Compositions must be pushed to openmc.lib before the search runs. - + The keff search is executed at the beginning of a depletion step, before the transport operator is called. Without an explicit update, both openmc.lib.materials and the operator's AtomNumber still hold the previous @@ -45,24 +156,26 @@ def test_materials_updated_before_search(control_and_calls): them -- freezing nuclide densities at their initial values. """ control, calls = control_and_calls - n = [np.array([1.0, 2.0, 3.0]), np.array([4.0, 5.0, 6.0])] + root = control.run(n) - + assert root == 0.5 assert [name for name, _ in calls] == [ 'update_materials', 'search', 'update_vec'] - + # The vector handed to the operator must be the current composition recorded = calls[0][1] assert len(recorded) == len(n) for actual, expected in zip(recorded, n): np.testing.assert_array_equal(actual, expected) - - + + def test_bracket_validation(): operator = MockOperator([]) + with pytest.raises(ValueError, match='exactly 2 elements'): _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [0.0]) + with pytest.raises(ValueError, match=r'bracket\[0\] must be'): _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [2.0, 1.0]) From 506fe932e3c72591587db52566beaac9d587e74e Mon Sep 17 00:00:00 2001 From: GuySten Date: Sat, 29 Aug 2026 21:02:12 +0300 Subject: [PATCH 3/4] remove unneeded fix --- tests/unit_tests/test_deplete_keff_search_control.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/unit_tests/test_deplete_keff_search_control.py b/tests/unit_tests/test_deplete_keff_search_control.py index a348e1b190f..a3df2337e5c 100755 --- a/tests/unit_tests/test_deplete_keff_search_control.py +++ b/tests/unit_tests/test_deplete_keff_search_control.py @@ -169,13 +169,3 @@ def test_materials_updated_before_search(control_and_calls): assert len(recorded) == len(n) for actual, expected in zip(recorded, n): np.testing.assert_array_equal(actual, expected) - - -def test_bracket_validation(): - operator = MockOperator([]) - - with pytest.raises(ValueError, match='exactly 2 elements'): - _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [0.0]) - - with pytest.raises(ValueError, match=r'bracket\[0\] must be'): - _KeffSearchControl(operator, lambda x: None, 0.0, 1.0, [2.0, 1.0]) From 99ce64eba3a4b39ab3a7b992d03be56c6915261d Mon Sep 17 00:00:00 2001 From: Paul Romano Date: Wed, 9 Sep 2026 14:59:54 -0500 Subject: [PATCH 4/4] Simplify keff search control tests and clean up review nits Address review feedback on the depletion/keff-search fix: - Replace the MockOperator class and single-use fixture in the unit test with a single unittest.mock.Mock, whose mock_calls records the global call ordering with arguments. Same coverage in a third of the lines, and asserting identity of the vector handed to the operator is a stronger check than the element-wise array comparison it replaces. - Revert the module docstring churn, drop 15 trailing-whitespace lines and restore the test file's 644 mode. - Trim the explanatory comment in _KeffSearchControl.run(). - Add a behavioral assertion to the regression test: with the bug, the depletion vector is reverted to its beginning-of-step values, so no fission products ever appear. Checking that Xe135 is nonzero at the final step catches this for all three parametrizations without needing reference data. Every fission product is exactly 0.0 at both steps in the current ref_depletion_with_*.h5 files, which were generated before the fix. Co-Authored-By: Claude Opus 5 (1M context) --- openmc/deplete/keff_search_control.py | 11 +-- .../deplete_with_keff_search_control/test.py | 6 ++ .../test_deplete_keff_search_control.py | 78 ++++++------------- 3 files changed, 34 insertions(+), 61 deletions(-) mode change 100755 => 100644 tests/unit_tests/test_deplete_keff_search_control.py diff --git a/openmc/deplete/keff_search_control.py b/openmc/deplete/keff_search_control.py index d9465a8a21a..5cd50ed6555 100644 --- a/openmc/deplete/keff_search_control.py +++ b/openmc/deplete/keff_search_control.py @@ -54,13 +54,10 @@ def run(self, x): root : float Parameter value that achieves target keff """ - # The keff search happens before the transport operator is called for - # this step, so both openmc.lib.materials and the operator's AtomNumber - # still hold the compositions from the previous operator call. Push the - # current beginning-of-step compositions in first, otherwise the search - # is performed on stale materials and _update_vec() below overwrites - # `x` with those stale densities, freezing the composition at its - # initial state for the entire depletion calculation. + # This runs before the operator is called for this step, so + # openmc.lib.materials and the operator's AtomNumber still hold the + # previous step's compositions. Push `x` in first, otherwise the search + # runs on stale materials and _update_vec() reverts `x` to them. self.operator._update_materials_and_nuclides(x) root = self._search_for_keff() diff --git a/tests/regression_tests/deplete_with_keff_search_control/test.py b/tests/regression_tests/deplete_with_keff_search_control/test.py index 82e6012809f..d326e69ad13 100644 --- a/tests/regression_tests/deplete_with_keff_search_control/test.py +++ b/tests/regression_tests/deplete_with_keff_search_control/test.py @@ -138,3 +138,9 @@ def test_keff_search_control(run_in_tmpdir, model, function, x0, x1, bracket, re # Use high tolerance here assert res_test[0].keff_search_root == pytest.approx(res_ref[0].keff_search_root, rel=2) + + # The keff search must not clobber the depleted compositions. If the search + # runs against the previous step's materials, the depletion vector is + # reverted to its beginning-of-step values and no fission products appear. + _, xe135 = res_test.get_atoms(model.materials[0], 'Xe135') + assert xe135[-1] > 0.0 diff --git a/tests/unit_tests/test_deplete_keff_search_control.py b/tests/unit_tests/test_deplete_keff_search_control.py old mode 100755 new mode 100644 index a3df2337e5c..b21397338d6 --- a/tests/unit_tests/test_deplete_keff_search_control.py +++ b/tests/unit_tests/test_deplete_keff_search_control.py @@ -1,6 +1,7 @@ -"""Tests for the KeffSearchControl class and openmc.deplete.keff_search_control.""" +""" Tests for KeffSearchControl class """ from pathlib import Path +from unittest.mock import Mock import pytest import numpy as np @@ -87,35 +88,6 @@ def set_u235_density(u235_density): return u235_density -class MockOperator: - """Minimal operator recording calls to _update_materials_and_nuclides.""" - - def __init__(self, calls): - self.calls = calls - - def _update_materials_and_nuclides(self, vec): - self.calls.append(('update_materials', [v.copy() for v in vec])) - - -@pytest.fixture -def control_and_calls(monkeypatch): - calls = [] - operator = MockOperator(calls) - control = _KeffSearchControl( - operator, lambda x: None, x0=0.0, x1=1.0, bracket=[0.0, 2.0]) - - def fake_search(): - calls.append(('search', None)) - return 0.5 - - def fake_update_vec(x): - calls.append(('update_vec', None)) - - monkeypatch.setattr(control, '_search_for_keff', fake_search) - monkeypatch.setattr(control, '_update_vec', fake_update_vec) - return control, calls - - @pytest.mark.parametrize("function, x0, x1, bracket", [ (translate_cell, -1.0, 1.0, (-5.0, 5.0)), (rotate_cell, -45.0, 45.0, (-90.0, 90.0)), @@ -144,28 +116,26 @@ def test_integrator_add_keff_search_control(run_in_tmpdir, function, x0, x1, bra assert integrator._keff_search_control.search_kwargs['x_max'] == bracket[1] assert integrator._keff_search_control.search_kwargs['k_tol'] == 0.1 assert not integrator._keff_search_control.search_kwargs['output'] - - -def test_materials_updated_before_search(control_and_calls): - """Compositions must be pushed to openmc.lib before the search runs. - - The keff search is executed at the beginning of a depletion step, before - the transport operator is called. Without an explicit update, both - openmc.lib.materials and the operator's AtomNumber still hold the previous - call's compositions, and _update_vec() overwrites the depleted vector with - them -- freezing nuclide densities at their initial values. + + +def test_materials_updated_before_search(monkeypatch): + """Test that compositions reach the operator before the keff search runs + + The search happens at the beginning of a depletion step, before the + transport operator is called. Without the update, the search uses the + previous step's materials and _update_vec() reverts the depletion vector + to them. """ - control, calls = control_and_calls - n = [np.array([1.0, 2.0, 3.0]), np.array([4.0, 5.0, 6.0])] - - root = control.run(n) - - assert root == 0.5 - assert [name for name, _ in calls] == [ - 'update_materials', 'search', 'update_vec'] - - # The vector handed to the operator must be the current composition - recorded = calls[0][1] - assert len(recorded) == len(n) - for actual, expected in zip(recorded, n): - np.testing.assert_array_equal(actual, expected) + recorder = Mock() + recorder.search.return_value = 0.5 + control = _KeffSearchControl(recorder.operator, lambda x: None, + x0=0.0, x1=1.0, bracket=[0.0, 2.0]) + monkeypatch.setattr(control, '_search_for_keff', recorder.search) + monkeypatch.setattr(control, '_update_vec', recorder.update_vec) + + n = [np.array([1.0, 2.0, 3.0])] + + assert control.run(n) == 0.5 + assert [c[0] for c in recorder.mock_calls] == [ + 'operator._update_materials_and_nuclides', 'search', 'update_vec'] + assert recorder.mock_calls[0].args[0] is n