adressing third round of comment from @pshriwise

include/openmc/surface.h

@@ -38,7 +38,6 @@ class Surface {
   int id_;                           //!< Unique ID
   std::string name_;                 //!< User-defined name
   unique_ptr<BoundaryCondition> bc_; //!< Boundary condition
-  GeometryType geom_type_;           //!< Geometry type indicator (CSG or DAGMC)
   bool surf_source_ {false}; //!< Activate source banking for the surface?
 
   explicit Surface(pugi::xml_node surf_node);
@@ -91,6 +90,13 @@ class Surface {
   //! Get the BoundingBox for this surface.
   virtual BoundingBox bounding_box(bool /*pos_side*/) const { return {}; }
 
+  // Accessors
+  const GeometryType& geom_type() const { return geom_type_; }
+  GeometryType& geom_type() { return geom_type_; }
+
+private:
+  GeometryType geom_type_; //!< Geometry type indicator (CSG or DAGMC)
+
 protected:
   virtual void to_hdf5_inner(hid_t group_id) const = 0;
 };

openmc/dagmc.py

@@ -20,6 +20,9 @@ class DAGMCUniverse(openmc.UniverseBase):
 
     .. versionadded:: 0.13.0
 
+    .. versionadded:: 0.15.1-dev
+        Moved this classe from openmc.universe to openmc.dagmc
+
     Parameters
     ----------
     filename : str
@@ -139,9 +142,11 @@ def material_overrides(self, val):
 
         self._material_overrides = val
 
-    def add_material_override(self, mat_name, overrides):
+    def add_material_override(self, mat_name=None, cell_id=None, overrides=None):
         """Add a material override to the universe.
 
+        .. versionadded:: 0.15
+
         Parameters
         ----------
         key : str
@@ -150,12 +155,27 @@ def add_material_override(self, mat_name, overrides):
             Material names to replace the key with
 
         """
-        cv.check_type('material name', mat_name, str)
-        if mat_name not in self.material_names:
-            raise ValueError(
-                f"Material name '{mat_name}' not found in DAGMC file")
+        key = ""
+        if mat_name and cell_id:
+            raise ValueError("Only one of 'mat_name' or 'cell_id' can be set")
+        elif cell_id:
+            cv.check_type('cell id', cell_id, int)
+            if cell_id not in self.cells:
+                raise ValueError(
+                    f"Cell ID '{cell_id}' not found in DAGMC universe")
+            else:
+                key = str(cell_id)
+        elif mat_name:
+            cv.check_type('material name', mat_name, str)
+            if mat_name not in self.material_names:
+                raise ValueError(
+                    f"Material name '{mat_name}' not found in DAGMC file")
+            else:
+                key = mat_name
+        else:
+            raise ValueError("Either 'mat_name' or 'cell_id' must be set")
+
         cv.check_iterable_type('material objects', overrides, str)
-
         self.material_overrides[mat_name] = overrides
 
     @filename.setter
@@ -281,7 +301,7 @@ def build_overide_mat_from_cells(self):
             None
         """
         for cell in self.cells.values():
-            if cell.n_instances > 1 and isinstance(cell.fill, Iterable):
+            if isinstance(cell.fill, Iterable):
                 for mat in cell.fill:
                     self.material_overrides[str(cell.id)] = [mat.name for mat in cell.fill]
 
@@ -489,10 +509,10 @@ def remove_cell(self, cell):
         # If the Cell is in the Universe's list of Cells, delete it
         self._cells.pop(cell.id, None)
 
-    def sync_dagmc_cells(self, mats={}):
+    def sync_dagmc_cells(self, mats):
         """Synchronize DAGMC cell information between Python and C API
 
-        .. versionadded:: 0.13.0
+        .. versionadded:: 0.15.1-dev
 
         """
         import openmc.lib
@@ -524,7 +544,7 @@ def sync_dagmc_cells(self, mats={}):
 
 class DAGMCCell(openmc.Cell):
     """
-    .. versionadded:: 0.13.2
+    .. versionadded:: 0.15.1-dev
     A cell class for DAGMC-based geometries.
 
     Parameters

openmc/model/model.py

@@ -330,16 +330,19 @@ def sync_dagmc_universe(self):
         Synchronize all DAGMC universes with the current geometry.
         This method iterates over all DAGMC universes in the geometry and
         synchronizes their cells with the current material assignments.
+
+        .. versionadded:: 0.15.1-dev
+
         Returns:
             None
         """
         if self.is_initialized:
             if self.materials:
-                mats = self.materials
+                materials = self.materials
             else:
-                mats = self.geometry.get_all_materials().values()
+                materials = self.geometry.get_all_materials().values()
             for dagmc_universe in self.geometry.get_all_dagmc_universes().values():
-                dagmc_universe.sync_dagmc_cells(mats)
+                dagmc_universe.sync_dagmc_cells(materials)
         else:
             raise ValueError("The model must be initialized before calling "
                              "this method")
@@ -1154,38 +1157,47 @@ def differentiate_depletable_mats(self, diff_volume_method : str = None):
 
         .. versionadded:: 0.14.0
 
+        .. version added:: 0.15.1-dev
+            diff_volume_method default is None, do not apply volume to the new
+            materials. Is now a convenience method for
+            differentiate_mats(diff_volume_method, depletable_only=True)
+
         Parameters
         ----------
         diff_volume_method : str
             Specifies how the volumes of the new materials should be found.
-            Default is to 'divide equally' which divides the original material
-            volume equally between the new materials, 'match cell' sets the
-            volume of the material to volume of the cell they fill.
+            Default is to 'None', do not apply volume to the new materials,
+            'divide equally' which divides the original material
+            volume equally between the new materials,
+            'match cell' sets the volume of the material to volume of the cell
+            they fill.
         """
         self.differentiate_mats(diff_volume_method, depletable_only=True)
 
     def differentiate_mats(self, diff_volume_method: str = None, depletable_only: bool = True):
-        """Assign distribmats for each depletable material
+        """Assign distribmats for each material
 
-        .. versionadded:: 0.14.0
+        .. versionadded:: 0.15.1-dev
 
         Parameters
         ----------
         diff_volume_method : str
             Specifies how the volumes of the new materials should be found.
-            Default is to 'divide equally' which divides the original material
-            volume equally between the new materials, 'match cell' sets the
-            volume of the material to volume of the cell they fill.
+            Default is to 'None', do not apply volume to the new materials, 
+            'divide equally' which divides the original material
+            volume equally between the new materials, 
+            'match cell' sets the volume of the material to volume of the cell 
+            they fill.
+        depletable_only : bool
+            Default is True, only depletable materials will be differentiated all materials will be 
+            differentiated otherwise.
         """
-        if diff_volume_method not in ("divide equally", "match cell", None):
-            raise ValueError(
-                "diff_volume_method must be 'divide equally' or 'match cell', "
-                f"not '{diff_volume_method}'"
-            )
+        check_value('volume differentiation method', diff_volume_method, ["divide equally", "match cell", None])
+
         # Count the number of instances for each cell and material
         self.geometry.determine_paths(instances_only=True)
 
-        # Extract all depletable materials which have multiple instances
+        # Extract all or depletable_only materials which have multiple instance
         distribmats = set(
             [mat for mat in self.materials
                 if (mat.depletable or not depletable_only) and mat.num_instances > 1])
@@ -1217,8 +1229,8 @@ def differentiate_mats(self, diff_volume_method: str = None, depletable_only: bo
                                 )
                             cell.fill.volume = cell.volume
                     if isinstance(cell, openmc.DAGMCCell):
-                        for i, f in enumerate(cell.fill):
-                            f.name += f"_{cell.id}_{i}"
+                        for i in range(cell.num_instances):
+                            cell.fill[i].name = f"{cell.fill[i].name}_{cell.id}_{i}"
 
         if self.materials is not None:
             self.materials = openmc.Materials(

src/dagmc.cpp

@@ -765,7 +765,7 @@ BoundingBox DAGCell::bounding_box() const
 DAGSurface::DAGSurface(std::shared_ptr<moab::DagMC> dag_ptr, int32_t dag_idx)
   : Surface {}, dagmc_ptr_(dag_ptr), dag_index_(dag_idx)
 {
-  geom_type_ = GeometryType::DAG;
+  geom_type() = GeometryType::DAG;
 } // empty constructor
 
 moab::EntityHandle DAGSurface::mesh_handle() const

src/particle.cpp

@@ -533,7 +533,7 @@ void Particle::cross_surface(const Surface& surf)
 
 // if we're crossing a CSG surface, make sure the DAG history is reset
 #ifdef DAGMC
-  if (surf.geom_type_ == GeometryType::CSG)
+  if (surf.geom_type() == GeometryType::CSG)
     history().reset();
 #endif
 
@@ -548,7 +548,7 @@ void Particle::cross_surface(const Surface& surf)
 
 #ifdef DAGMC
   // in DAGMC, we know what the next cell should be
-  if (surf.geom_type_ == GeometryType::DAG) {
+  if (surf.geom_type() == GeometryType::DAG) {
     int32_t i_cell = next_cell(std::abs(surface()), cell_last(n_coord() - 1),
                        lowest_coord().universe) -
                      1;
@@ -668,7 +668,8 @@ void Particle::cross_reflective_bc(const Surface& surf, Direction new_u)
   // the lower universes.
   // (unless we're using a dagmc model, which has exactly one universe)
   n_coord() = 1;
-  if (surf.geom_type_ != GeometryType::DAG && !neighbor_list_find_cell(*this)) {
+  if (surf.geom_type() != GeometryType::DAG &&
+      !neighbor_list_find_cell(*this)) {
     mark_as_lost("Couldn't find particle after reflecting from surface " +
                  std::to_string(surf.id_) + ".");
     return;

src/plot.cpp

@@ -1301,7 +1301,7 @@ void ProjectionPlot::create_output() const
 
             int32_t i_surface = std::abs(p.surface()) - 1;
             if (i_surface > 0 &&
-                model::surfaces[i_surface]->geom_type_ == GeometryType::DAG) {
+                model::surfaces[i_surface]->geom_type() == GeometryType::DAG) {
 #ifdef DAGMC
               int32_t i_cell = next_cell(i_surface,
                 p.cell_last(p.n_coord() - 1), p.lowest_coord().universe);

src/surface.cpp

@@ -165,9 +165,9 @@ void Surface::to_hdf5(hid_t group_id) const
 {
   hid_t surf_group = create_group(group_id, fmt::format("surface {}", id_));
 
-  if (geom_type_ == GeometryType::DAG) {
+  if (geom_type() == GeometryType::DAG) {
     write_string(surf_group, "geom_type", "dagmc", false);
-  } else if (geom_type_ == GeometryType::CSG) {
+  } else if (geom_type() == GeometryType::CSG) {
     write_string(surf_group, "geom_type", "csg", false);
 
     if (bc_) {
@@ -189,11 +189,11 @@ void Surface::to_hdf5(hid_t group_id) const
 
 CSGSurface::CSGSurface() : Surface {}
 {
-  geom_type_ = GeometryType::CSG;
+  geom_type() = GeometryType::CSG;
 };
 CSGSurface::CSGSurface(pugi::xml_node surf_node) : Surface {surf_node}
 {
-  geom_type_ = GeometryType::CSG;
+  geom_type() = GeometryType::CSG;
 };
 
 //==============================================================================