Respond to reviewer comments on FY helpers

- Change Operator attribute name for fission yield helper
- Use copy.deepcopy for constant_yields, fast_yields, and thermal_yields
- Change import location of openmc in tests
- Use bisect.bisect_left to find replacement fission yields on
  FissionYieldCutoffHelper if thermal or fast energies not found

openmc/deplete/abc.py

@@ -396,10 +396,7 @@ class FissionYieldHelper(ABC):
 
     @property
     def constant_yields(self):
-        out = {}
-        for key, sub in self._constant_yields.items():
-            out[key] = sub.copy()
-        return out
+        return deepcopy(self._constant_yields)
 
     @abstractmethod
     def weighted_yields(self, local_mat_index):

openmc/deplete/chain.py

@@ -663,11 +663,8 @@ class Chain(object):
 
     @fission_yields.setter
     def fission_yields(self, yields):
-        if yields is None:
-            self._fission_yields = None
-            return
-        if isinstance(yields, Mapping):
-            self._fission_yields = [yields]
-            return
-        check_type("fission_yields", yields, Iterable, Mapping)
+        if yields is not None:
+            if isinstance(yields, Mapping):
+                yields = [yields]
+            check_type("fission_yields", yields, Iterable, Mapping)
         self._fission_yields = yields

openmc/deplete/helpers.py

@@ -1,9 +1,10 @@
 """
 Class for normalizing fission energy deposition
 """
+from copy import deepcopy
 from itertools import product
 from numbers import Real
-import operator
+import bisect
 
 from numpy import dot, zeros, newaxis
 
@@ -200,8 +201,8 @@ class ConstantFissionYieldHelper(FissionYieldHelper):
             distances = [abs(energy - ene) for ene in nuc.yield_energies]
             min_index = min(
                 range(len(nuc.yield_energies)), key=distances.__getitem__)
-            self._constant_yields[name] = (
-                nuc.yield_data[nuc.yield_energies[min_index]])
+            min_E = min(nuc.yield_energies, key=lambda e: abs(e - energy))
+            self._constant_yields[name] = nuc.yield_data[min_E]
 
     @classmethod
     def from_operator(cls, operator, **kwargs):
@@ -310,17 +311,21 @@ class FissionYieldCutoffHelper(TalliedFissionYieldHelper):
             yields = nuc.yield_data
             energies = nuc.yield_energies
             thermal = yields.get(thermal_energy)
-            if thermal is None:
-                # find first index >= cutoff
-                ix = self._find_fallback_energy(
-                    name, energies, cutoff, True)
-                thermal = yields[energies[ix - 1]]
             fast = yields.get(fast_energy)
-            if fast is None:
-                # find first index <= cutoff
-                rev_ix = self._find_fallback_energy(
-                    name, reversed(energies), cutoff, False)
-                fast = yields[energies[-rev_ix]]
+            if thermal is None or fast is None:
+                insert_ix = bisect.bisect_left(energies, cutoff)
+                # if zero, then all energies > cutoff
+                # if len(energies), then all energies <= cutoff
+                if insert_ix == 0 or insert_ix == len(energies):
+                    tail = map("{:5.3e}".format, energies)
+                    raise ValueError(
+                        "Cannot find replacement fission yields for {} given "
+                        "cutoff {:5.3e} eV. Yields provided at [{}] eV".format(
+                            name, cutoff, ", ".join(tail)))
+                if thermal is None:
+                    thermal = yields[energies[insert_ix - 1]]
+                if fast is None:
+                    fast = yields[energies[insert_ix]]
             self._thermal_yields[name] = thermal
             self._fast_yields[name] = fast
 
@@ -346,20 +351,6 @@ class FissionYieldCutoffHelper(TalliedFissionYieldHelper):
         return cls(operator.chain.nuclides, len(operator.burnable_mats),
                    **kwargs)
 
-    @staticmethod
-    def _find_fallback_energy(name, energies, cutoff, check_under):
-        cutoff_func = operator.ge if check_under else operator.le
-        found = False
-        for ix, ene in enumerate(energies):
-            if cutoff_func(ene, cutoff):
-                found = True
-                break
-        if found and ix != 0:
-            return ix
-        domain = "thermal" if check_under else "fast"
-        raise ValueError("Could not find {} yields for {} "
-                         "with cutoff {} eV".format(domain, name, cutoff))
-
     def generate_tallies(self, materials, mat_indexes):
         """Use C API to produce a fission rate tally in burnable materials
 
@@ -435,17 +426,11 @@ class FissionYieldCutoffHelper(TalliedFissionYieldHelper):
 
     @property
     def thermal_yields(self):
-        out = {}
-        for key, sub in self._thermal_yields.items():
-            out[key] = sub.copy()
-        return out
+        return deepcopy(self._thermal_yields)
 
     @property
     def fast_yields(self):
-        out = {}
-        for key, sub in self._fast_yields.items():
-            out[key] = sub.copy()
-        return out
+        return deepcopy(self._fast_yields)
 
 
 class AveragedFissionYieldHelper(TalliedFissionYieldHelper):

openmc/deplete/operator.py

@@ -137,7 +137,7 @@ class Operator(TransportOperator):
     diff_burnable_mats : bool
         Whether to differentiate burnable materials with multiple instances
     """
-    _fission_helpers_ = {
+    _fission_helpers = {
         "average": AveragedFissionYieldHelper,
         "constant": ConstantFissionYieldHelper,
         "cutoff": FissionYieldCutoffHelper,
@@ -147,10 +147,10 @@ class Operator(TransportOperator):
                  diff_burnable_mats=False, fission_q=None,
                  dilute_initial=1.0e3, fission_yield_mode="constant",
                  fission_yield_opts=None):
-        if fission_yield_mode not in self._fission_helpers_:
+        if fission_yield_mode not in self._fission_helpers:
             raise KeyError(
                 "fission_yield_mode must be one of {}, not {}".format(
-                    ", ".join(self._fission_helpers_), fission_yield_mode))
+                    ", ".join(self._fission_helpers), fission_yield_mode))
         super().__init__(chain_file, fission_q, dilute_initial, prev_results)
         self.round_number = False
         self.prev_res = None
@@ -207,10 +207,10 @@ class Operator(TransportOperator):
         self._energy_helper = ChainFissionHelper()
 
         # Select and create fission yield helper
-        fission_helper = self._fission_helpers_[fission_yield_mode]
+        fission_helper = self._fission_helpers[fission_yield_mode]
         fission_yield_opts = (
             {} if fission_yield_opts is None else fission_yield_opts)
-        self._fsn_yield_helper = fission_helper.from_operator(
+        self._yield_helper = fission_helper.from_operator(
             self, **fission_yield_opts)
 
     def __call__(self, vec, power):
@@ -242,7 +242,7 @@ class Operator(TransportOperator):
         nuclides = self._get_tally_nuclides()
         self._rate_helper.nuclides = nuclides
         self._energy_helper.nuclides = nuclides
-        self._fsn_yield_helper.update_tally_nuclides(nuclides)
+        self._yield_helper.update_tally_nuclides(nuclides)
 
         # Run OpenMC
         openmc.capi.reset()
@@ -448,8 +448,8 @@ class Operator(TransportOperator):
             self.chain.nuclides, self.reaction_rates.index_nuc, materials)
         # Tell fission yield helper what materials this process is
         # responsible for
-        self._fsn_yield_helper.generate_tallies(
-                materials, tuple(sorted(self._mat_index_map.values())))
+        self._yield_helper.generate_tallies(
+            materials, tuple(sorted(self._mat_index_map.values())))
 
         # Return number density vector
         return list(self.number.get_mat_slice(np.s_[:]))
@@ -595,7 +595,7 @@ class Operator(TransportOperator):
         # Keep track of energy produced from all reactions in eV per source
         # particle
         self._energy_helper.reset()
-        self._fsn_yield_helper.unpack()
+        self._yield_helper.unpack()
 
         # Store fission yield dictionaries
         fission_yields = []
@@ -622,7 +622,7 @@ class Operator(TransportOperator):
                 mat_index, nuc_ind, react_ind)
 
             # Compute fission yields for this material
-            fission_yields.append(self._fsn_yield_helper.weighted_yields(i))
+            fission_yields.append(self._yield_helper.weighted_yields(i))
 
             # Accumulate energy from fission
             self._energy_helper.update(tally_rates[:, fission_ind], mat_index)

tests/unit_tests/test_deplete_fission_yields.py

@@ -5,7 +5,6 @@ from unittest.mock import Mock
 
 import pytest
 import numpy
-
 from openmc.deplete.nuclide import Nuclide, FissionYieldDistribution
 from openmc.deplete.helpers import (
     FissionYieldCutoffHelper, ConstantFissionYieldHelper,
@@ -75,10 +74,10 @@ def test_cutoff_construction(nuclide_bundle):
         "U235": nuclide_bundle.u235.yield_data[0.0253]}
     assert helper.fast_yields == {"U235": nuclide_bundle.u235.yield_data[5e5]}
     # test failures in cutoff: super low, super high
-    with pytest.raises(ValueError, match="thermal yields"):
+    with pytest.raises(ValueError, match="replacement fission yields"):
         FissionYieldCutoffHelper(
             nuclide_bundle, 1, thermal_energy=0.001, cutoff=0.002)
-    with pytest.raises(ValueError, match="fast yields"):
+    with pytest.raises(ValueError, match="replacement fission yields"):
         FissionYieldCutoffHelper(
             nuclide_bundle, 1, cutoff=15e6, fast_energy=17e6)
 

tests/unit_tests/test_deplete_nuclide.py

@@ -4,7 +4,6 @@ import xml.etree.ElementTree as ET
 
 import numpy
 import pytest
-
 from openmc.deplete import nuclide