addressed some of the requested changes from PR

docs/source/pythonapi/base.rst

@@ -118,6 +118,7 @@ Constructing Tallies
    openmc.CellFromFilter
    openmc.CellbornFilter
    openmc.CellInstanceFilter
+   openmc.CollisionFilter
    openmc.SurfaceFilter
    openmc.MeshFilter
    openmc.MeshSurfaceFilter

include/openmc/tallies/filter_collision.h

@@ -2,7 +2,7 @@
 #define OPENMC_TALLIES_FILTER_COLLISIONS_H
 
 #include <vector>
-
+#include <unordered_map>
 #include <gsl/gsl>
 
 #include "openmc/tallies/filter.h"
@@ -45,6 +45,9 @@ protected:
   // Data members
 
   std::vector<int> bins_;
+
+  std::unordered_map<int,int> map_;
+
 };
 
 } // namespace openmc

openmc/filter.py

@@ -970,7 +970,7 @@ class CollisionFilter(Filter):
 
     Parameters
     ----------
-    values : Iterable of Int
+    bins : Iterable of int
         A list or iterable of the number of collisions, as integer values. 
         The events whose post-scattering collision number equals one of 
         the provided values will be counted. 
@@ -979,28 +979,24 @@ class CollisionFilter(Filter):
 
     Attributes
     ----------
-    values : numpy.ndarray
-        An array of integer values representing the number of collisions events
-        by which to filter
     id : int
         Unique identifier for the filter
     bins : numpy.ndarray
         An array of integer values representing the number of collisions events
         by which to filter
-    num_bins : Integral
+    num_bins : int
         The number of filter bins
 
     """
 
-    def __init__(self, values, filter_id=None):
-        self.values = np.asarray(values)
-        self.bins = self.values
+    def __init__(self, bins, filter_id=None):
+        self.bins = np.asarray(bins)
         self.id = filter_id
 
 
     def __repr__(self):
         string = type(self).__name__ + '\n'
-        string += '{: <16}=\t{}\n'.format('\tValues', self.values)
+        string += '{: <16}=\t{}\n'.format('\tValues', self.bins)
         string += '{: <16}=\t{}\n'.format('\tID', self.id)
         return string
 
@@ -1014,62 +1010,6 @@ class CollisionFilter(Filter):
             cv.check_type('filter value', x, Integral)
             cv.check_greater_than('filter value', x, 0, equality=True)
 
-    def get_bin_index(self, filter_bin):
-        i = np.where(self.bins[:, 1] == filter_bin[1])[0]
-        if len(i) == 0:
-            msg = 'Unable to get the bin index for Filter since "{0}" ' \
-                  'is not one of the bins'.format(filter_bin)
-            raise ValueError(msg)
-        else:
-            return i[0]
-
-    def get_pandas_dataframe(self, data_size, stride, **kwargs):
-        """Builds a Pandas DataFrame for the Filter's bins.
-
-        This method constructs a Pandas DataFrame object for the filter with
-        columns annotated by filter bin information. This is a helper method for
-        :meth:`Tally.get_pandas_dataframe`.
-
-        Parameters
-        ----------
-        data_size : int
-            The total number of bins in the tally corresponding to this filter
-        stride : int
-            Stride in memory for the filter
-
-        Returns
-        -------
-        pandas.DataFrame
-            A Pandas DataFrame with one column of the lower energy bound and one
-            column of upper energy bound for each filter bin.  The number of
-            rows in the DataFrame is the same as the total number of bins in the
-            corresponding tally, with the filter bin appropriately tiled to map
-            to the corresponding tally bins.
-
-        See also
-        --------
-        Tally.get_pandas_dataframe(), CrossFilter.get_pandas_dataframe()
-
-        """
-        # Initialize Pandas DataFrame
-        df = pd.DataFrame()
-
-        # Extract the lower and upper energy bounds, then repeat and tile
-        # them as necessary to account for other filters.
-        bins = np.repeat(self.bins[:], stride)
-        tile_factor = data_size // len(bins)
-        bins = np.tile(bins, tile_factor)
-
-        # Add the new energy columns to the DataFrame.
-        if hasattr(self, 'units'):
-            units = ' [{}]'.format(self.units)
-        else:
-            units = ''
-
-        df.loc[:, self.short_name.lower() + ' collisions' ] = bins
-
-        return df
-
     def to_xml_element(self):
         """Return XML Element representing the Filter.
 
@@ -1080,12 +1020,11 @@ class CollisionFilter(Filter):
 
         """
         element = super().to_xml_element()
-        element[0].text = ' '.join(str(x) for x in self.values)
+        element[0].text = ' '.join(str(x) for x in self.bins)
         return element
 
 
 
-
 class RealFilter(Filter):
     """Tally modifier that describes phase-space and other characteristics
 

openmc/lib/filter.py

@@ -16,10 +16,10 @@ from .mesh import RegularMesh
 
 __all__ = [
     'Filter', 'AzimuthalFilter', 'CellFilter', 'CellbornFilter', 'CellfromFilter',
-    'CellInstanceFilter', 'DistribcellFilter', 'DelayedGroupFilter', 'EnergyFilter',
-    'EnergyoutFilter', 'EnergyFunctionFilter', 'LegendreFilter', 'MaterialFilter',
-    'MeshFilter', 'MeshSurfaceFilter', 'MuFilter', 'ParticleFilter', 'PolarFilter',
-    'SphericalHarmonicsFilter', 'SpatialLegendreFilter', 'SurfaceFilter',
+    'CellInstanceFilter', 'CollisionFilter', 'DistribcellFilter', 'DelayedGroupFilter', 
+    'EnergyFilter', 'EnergyoutFilter', 'EnergyFunctionFilter', 'LegendreFilter', 
+    'MaterialFilter', 'MeshFilter', 'MeshSurfaceFilter', 'MuFilter', 'ParticleFilter', 
+    'PolarFilter', 'SphericalHarmonicsFilter', 'SpatialLegendreFilter', 'SurfaceFilter',
     'UniverseFilter', 'ZernikeFilter', 'ZernikeRadialFilter', 'filters'
 ]
 
@@ -176,6 +176,10 @@ class EnergyFilter(Filter):
             self._index, len(energies), energies_p)
 
 
+class CollisionFilterFilter(CollisionFilter):
+    filter_type = 'collision'
+
+
 class EnergyoutFilter(EnergyFilter):
     filter_type = 'energyout'
 

src/tallies/filter.cpp

@@ -15,11 +15,11 @@
 #include "openmc/tallies/filter_cellborn.h"
 #include "openmc/tallies/filter_cellfrom.h"
 #include "openmc/tallies/filter_cell_instance.h"
+#include "openmc/tallies/filter_collision.h"
 #include "openmc/tallies/filter_delayedgroup.h"
 #include "openmc/tallies/filter_distribcell.h"
 #include "openmc/tallies/filter_energyfunc.h"
 #include "openmc/tallies/filter_energy.h"
-#include "openmc/tallies/filter_collision.h"
 #include "openmc/tallies/filter_legendre.h"
 #include "openmc/tallies/filter_material.h"
 #include "openmc/tallies/filter_mesh.h"

src/tallies/filter_collision.cpp

@@ -3,8 +3,6 @@
 #include <fmt/core.h>
 
 #include "openmc/capi.h"
-#include "openmc/constants.h" // For F90_NONE
-#include "openmc/mgxs_interface.h"
 #include "openmc/search.h"
 #include "openmc/settings.h"
 #include "openmc/xml_interface.h"
@@ -26,14 +24,12 @@ void CollisionFilter::set_bins(gsl::span<const int> bins)
   // Clear existing bins
   bins_.clear();
   bins_.reserve(bins.size());
+  map_.clear();
 
-  // Copy bins, ensuring they are valid
+  // Copy bins
   for (gsl::index i = 0; i < bins.size(); ++i) {
-    if (i > 0 && bins[i] <= bins[i - 1]) {
-      throw std::runtime_error {
-        "Number of Collisions bins must be monotonically increasing."};
-    }
     bins_.push_back(bins[i]);
+    map_[bins[i]] = i;
   }
 
   n_bins_ = bins_.size();
@@ -47,13 +43,10 @@ void CollisionFilter::get_all_bins(
 
   // Bin the collision number. Must fit exactly the desired collision number .
   if (n >= bins_.front() && n <= bins_.back()) {
-    auto it = find(bins_.begin(), bins_.end(), n);
-    if (it != bins_.end()) {
-      size_t bin = it - bins_.begin();
-      if (int(bins_[bin]) == n) {
-        match.bins_.push_back(bin);
-        match.weights_.push_back(1.0);
-      }
+    auto search = map_.find(n);
+    if (search != map_.end()){
+      match.bins_.push_back(search->second);
+      match.weights_.push_back(1.0);
     }
   }
 }
@@ -66,56 +59,7 @@ void CollisionFilter::to_statepoint(hid_t filter_group) const
 
 std::string CollisionFilter::text_label(int bin) const
 {
-  return fmt::format("Collision Number {}", int(bins_[bin]));
-}
-
-//==============================================================================
-// C-API functions
-//==============================================================================
-
-extern "C" int openmc_collision_filter_get_bins(
-  int32_t index, const int** energies, size_t* n)
-{
-  // Make sure this is a valid index to an allocated filter.
-  if (int err = verify_filter(index)) 
-    return err;
-
-  // Get a pointer to the filter and downcast.
-  const auto& filt_base = model::tally_filters[index].get();
-  auto* filt = dynamic_cast<CollisionFilter*>(filt_base);
-
-  // Check the filter type.
-  if (!filt) {
-    set_errmsg("Tried to get collision bins on a non-collision filter.");
-    return OPENMC_E_INVALID_TYPE;
-  }
-
-  // Output the bins.
-  *energies = filt->bins().data();
-  *n = filt->bins().size();
-  return 0;
-}
-
-extern "C" int openmc_collision_filter_set_bins(
-  int32_t index, size_t n, const int* energies)
-{
-  // Make sure this is a valid index to an allocated filter.
-  if (int err = verify_filter(index)) 
-    return err;
-
-  // Get a pointer to the filter and downcast.
-  const auto& filt_base = model::tally_filters[index].get();
-  auto* filt = dynamic_cast<CollisionFilter*>(filt_base);
-
-  // Check the filter type.
-  if (!filt) {
-    set_errmsg("Tried to set collision bins on a non-collision filter.");
-    return OPENMC_E_INVALID_TYPE;
-  }
-
-  // Update the filter.
-  filt->set_bins({energies, n});
-  return 0;
+  return fmt::format("Collision Number {}", bins_[bin]);
 }
 
 } // namespace openmc