Repository: openmc-dev/openmc

Issue:
Transport-corrected MGXS generation with merging broken
<!--
If you are a user of OpenMC and are running into trouble with the code or are
seeking general user support, we highly recommend posting on the OpenMC
discourse forum first. GitHub issues should be used specifically for bug reports
and feature requests.

https://openmc.discourse.group/

-->

## Bug Description

This exception occurs when generating transport-corrected MGXS with merged tallies:

```

File ~/.local/lib/python3.11/site-packages/openmc-0.14.1.dev0-py3.11-linux-x86_64.egg/openmc/checkvalue.py:42, in check_type(name, value, expected_type, expected_iter_type, none_ok)
     39     else:
     40         msg = (f'Unable to set "{name}" to "{value}" which is not of type "'
     41                f'{expected_type.__name__}"')
---> 42     raise TypeError(msg)
     44 if expected_iter_type:
     45     if isinstance(value, np.ndarray):

TypeError: Unable to set "expansion order" to "['P0' 'P1']" which is not of type "Integral"
```


## Steps to Reproduce
<!--Steps to reproduce the behavior (input file, or modifications to an existing input file, etc.)-->

Get a copy of the `mg-mode-part-ii` notebook from the examples. Comment out the scattering order to generate TCP0 cross sections. Set the MGXS types to:

```
# Specify multi-group cross section types to compute
mgxs_lib.mgxs_types = ['total', 'transport', 'absorption', 'nu-fission', 'fission',
                       'nu-scatter matrix', 'multiplicity matrix', 'chi', 'consistent scatter matrix']
```

The above exception will come up.

# Possible solutions

The problem is that the merge method assumes that the "bins" are what should be passed to the constructor of the newly merged tally, but for ExpansionFilters, it should actually be the highest order of the two.
So, there are two possible ways to solve this.
Add custom logic to check if we're dealing with an expansion filter in Filter.merge and use filter.order as the first argument to the constructor, otherwise filter.bins.
Make the ExpansionFilter take bins as an argument instead, e.g. ['P0', 'P1'] instead of 1. Now, that's a lot less clean, but at least it's not hacky.


Discussion:

Patch:
diff --git a/openmc/deplete/abc.py b/openmc/deplete/abc.py
index 1dea8440c..a574d4855 100644
--- a/openmc/deplete/abc.py
+++ b/openmc/deplete/abc.py
@@ -769,7 +769,7 @@ class Integrator(ABC):
         self.operator.write_bos_data(step_index + self._i_res)
         return x, res
 
-    def _get_bos_data_from_restart(self, step_index, source_rate, bos_conc):
+    def _get_bos_data_from_restart(self, source_rate, bos_conc):
         """Get beginning of step concentrations, reaction rates from restart"""
         res = self.operator.prev_res[-1]
         # Depletion methods expect list of arrays
@@ -823,7 +823,7 @@ class Integrator(ABC):
                 if i > 0 or self.operator.prev_res is None:
                     n, res = self._get_bos_data_from_operator(i, source_rate, n)
                 else:
-                    n, res = self._get_bos_data_from_restart(i, source_rate, n)
+                    n, res = self._get_bos_data_from_restart(source_rate, n)
 
                 # Solve Bateman equations over time interval
                 proc_time, n_list, res_list = self(n, res.rates, dt, source_rate, i)
@@ -1030,7 +1030,7 @@ class SIIntegrator(Integrator):
                     if self.operator.prev_res is None:
                         n, res = self._get_bos_data_from_operator(i, p, n)
                     else:
-                        n, res = self._get_bos_data_from_restart(i, p, n)
+                        n, res = self._get_bos_data_from_restart(p, n)
                 else:
                     # Pull rates, k from previous iteration w/o
                     # re-running transport
diff --git a/openmc/filter_expansion.py b/openmc/filter_expansion.py
index b9860846f..f8e677578 100644
--- a/openmc/filter_expansion.py
+++ b/openmc/filter_expansion.py
@@ -53,6 +53,34 @@ class ExpansionFilter(Filter):
         order = int(elem.find('order').text)
         return cls(order, filter_id=filter_id)
 
+    def merge(self, other):
+        """Merge this filter with another.
+
+        This overrides the behavior of the parent Filter class, since its
+        merging technique is to take the union of the set of bins of each
+        filter. That technique does not apply to expansion filters, since the
+        argument should be the maximum filter order rather than the list of all
+        bins.
+
+        Parameters
+        ----------
+        other : openmc.Filter
+            Filter to merge with
+
+        Returns
+        -------
+        merged_filter : openmc.Filter
+            Filter resulting from the merge
+
+        """
+
+        if not self.can_merge(other):
+            msg = f'Unable to merge "{type(self)}" with "{type(other)}"'
+            raise ValueError(msg)
+
+        # Create a new filter with these bins and a new auto-generated ID
+        return type(self)(max(self.order, other.order))
+
 
 class LegendreFilter(ExpansionFilter):
     r"""Score Legendre expansion moments up to specified order.
diff --git a/openmc/stats/univariate.py b/openmc/stats/univariate.py
index b33f88af8..e7ff64257 100644
--- a/openmc/stats/univariate.py
+++ b/openmc/stats/univariate.py
@@ -280,6 +280,9 @@ class Discrete(Univariate):
         Discrete distribution with low-importance points removed
 
         """
+        cv.check_less_than("tolerance", tolerance, 1.0, equality=True)
+        cv.check_greater_than("tolerance", tolerance, 0.0, equality=True)
+
         # Determine (reversed) sorted order of probabilities
         intensity = self.p * self.x
         index_sort = np.argsort(intensity)[::-1]


Test patch:
diff --git a/tests/unit_tests/test_stats.py b/tests/unit_tests/test_stats.py
index 8438535ff..f5368e912 100644
--- a/tests/unit_tests/test_stats.py
+++ b/tests/unit_tests/test_stats.py
@@ -89,6 +89,12 @@ def test_clip_discrete():
     d_same = d.clip(1e-6, inplace=True)
     assert d_same is d
 
+    with pytest.raises(ValueError):
+        d.clip(-1.)
+    
+    with pytest.raises(ValueError):
+        d.clip(5)
+
 
 def test_uniform():
     a, b = 10.0, 20.0