Address openmc-dev#1045 comments

docs/source/io_formats/summary.rst

@@ -34,7 +34,7 @@ The current version of the summary file format is 6.0.
              'material', 'universe', or 'lattice'.
            - **material** (*int* or *int[]*) -- Unique ID of the material(s)
              assigned to the cell. This dataset is present only if fill_type is
-             set to 'normal'.  The value '-2' signifies void material.  The data
+             set to 'normal'.  The value '-1' signifies void material.  The data
              is an array if the cell uses distributed materials, otherwise it is
              a scalar.
            - **temperature** (*double[]*) -- Temperature of the cell in Kelvin.

openmc/capi/cell.py

@@ -107,24 +107,27 @@ def fill(self):
         if fill_type.value == 1:
             if n.value > 1:
                 #TODO: off-by-one
-                return [Material(index=i+1) for i in indices[:n.value]]
+                return [Material(index=i+1 if i >=0 else i)
+                        for i in indices[:n.value]]
             else:
-                return Material(index=indices[0]+1)
+                #TODO: off-by-one
+                index = indices[0] + 1 if indices[0] >= 0 else indices[0]
+                return Material(index=index)
         else:
             raise NotImplementedError
 
     @fill.setter
     def fill(self, fill):
         if isinstance(fill, Iterable):
             n = len(fill)
-            indices = (c_int32*n)(*(m._index if m is not None else -2
+            indices = (c_int32*n)(*(m._index if m is not None else -1
                                     for m in fill))
             _dll.openmc_cell_set_fill(self._index, 1, n, indices)
         elif isinstance(fill, Material):
             indices = (c_int32*1)(fill._index)
             _dll.openmc_cell_set_fill(self._index, 1, 1, indices)
         elif fill is None:
-            indices = (c_int32*1)(-2)
+            indices = (c_int32*1)(-1)
             _dll.openmc_cell_set_fill(self._index, 1, 1, indices)
 
     def set_temperature(self, T, instance=None):

src/cell.cpp

@@ -199,33 +199,48 @@ generate_rpn(int32_t cell_id, std::vector<int32_t> infix)
 Cell::Cell(pugi::xml_node cell_node)
 {
   if (check_for_node(cell_node, "id")) {
-    id = stoi(get_node_value(cell_node, "id"));
+    id = std::stoi(get_node_value(cell_node, "id"));
   } else {
     fatal_error("Must specify id of cell in geometry XML file.");
   }
 
   if (check_for_node(cell_node, "name")) {
-    name = get_node_value(cell_node, "name", false);
+    name = get_node_value(cell_node, "name");
   }
 
   if (check_for_node(cell_node, "universe")) {
-    universe = stoi(get_node_value(cell_node, "universe"));
+    universe = std::stoi(get_node_value(cell_node, "universe"));
   } else {
     universe = 0;
   }
 
-  if (check_for_node(cell_node, "fill")) {
-    fill = stoi(get_node_value(cell_node, "fill"));
+  // Make sure that either material or fill was specified, but not both.
+  bool fill_present = check_for_node(cell_node, "fill");
+  bool material_present = check_for_node(cell_node, "material");
+  if (!(fill_present || material_present)) {
+    std::stringstream err_msg;
+    err_msg << "Neither material nor fill was specified for cell " << id;
+    fatal_error(err_msg);
+  }
+  if (fill_present && material_present) {
+    std::stringstream err_msg;
+    err_msg << "Cell " << id << " has both a material and a fill specified; "
+            << "only one can be specified per cell";
+    fatal_error(err_msg);
+  }
+
+  if (fill_present) {
+    fill = std::stoi(get_node_value(cell_node, "fill"));
   } else {
     fill = C_NONE;
   }
 
   // Read the material element.  There can be zero materials (filled with a
   // universe), more than one material (distribmats), and some materials may
   // be "void".
-  if (check_for_node(cell_node, "material")) {
+  if (material_present) {
     std::vector<std::string> mats
-         {get_node_array<std::string>(cell_node, "material")};
+         {get_node_array<std::string>(cell_node, "material", true)};
     if (mats.size() > 0) {
       material.reserve(mats.size());
       for (std::string mat : mats) {
@@ -235,28 +250,12 @@ Cell::Cell(pugi::xml_node cell_node)
           material.push_back(std::stoi(mat));
         }
       }
+      material.shrink_to_fit();
     } else {
-      material.push_back(C_NONE);
+      std::stringstream err_msg;
+      err_msg << "An empty material element was specified for cell " << id;
+      fatal_error(err_msg);
     }
-    material.shrink_to_fit();
-  } else {
-    material.push_back(C_NONE);
-    material.shrink_to_fit();
-  }
-
-  // Make sure that either material or fill was specified.
-  if ((material[0] == C_NONE) && (fill == C_NONE)) {
-    std::stringstream err_msg;
-    err_msg << "Neither material nor fill was specified for cell " << id;
-    fatal_error(err_msg);
-  }
-
-  // Make sure that material and fill haven't been specified simultaneously.
-  if ((material[0] != C_NONE) && (fill != C_NONE)) {
-    std::stringstream err_msg;
-    err_msg << "Cell " << id << " has both a material and a fill specified; "
-            << "only one can be specified per cell";
-    fatal_error(err_msg);
   }
 
   // Read the region specification.
@@ -562,7 +561,6 @@ extern "C" {
   int32_t cell_material(Cell* c, int i)
   {
     int32_t mat = c->material[i-1];
-    if (mat == C_NONE) return F90_NONE;
     if (mat == MATERIAL_VOID) return MATERIAL_VOID;
     return mat + 1;
   }

src/constants.F90

@@ -129,7 +129,7 @@ module constants
   integer(C_INT), bind(C, name='FILL_LATTICE')  :: FILL_LATTICE_C  = FILL_LATTICE
 
   ! Void material
-  integer, parameter :: MATERIAL_VOID = -2
+  integer, parameter :: MATERIAL_VOID = -1
 
   ! Flag to say that the outside of a lattice is not defined
   integer, parameter :: NO_OUTER_UNIVERSE = -1

src/constants.h

@@ -112,7 +112,7 @@ constexpr char SUBSHELLS[][4]  {
 
 // Void material
 // TODO: refactor and remove
-constexpr int MATERIAL_VOID {-2};
+constexpr int MATERIAL_VOID {-1};
 
 // ============================================================================
 // CROSS SECTION RELATED CONSTANTS
@@ -434,7 +434,6 @@ constexpr int DIFF_NUCLIDE_DENSITY {2};
 constexpr int DIFF_TEMPERATURE {3};
 
 constexpr int C_NONE {-1};
-constexpr int F90_NONE {0};
 
 // Interpolation rules
 enum class Interpolation {

src/geometry_aux.cpp

@@ -20,7 +20,7 @@ adjust_indices()
 {
   // Adjust material/fill idices.
   for (Cell* c : global_cells) {
-    if (c->material[0] == C_NONE) {
+    if (c->fill != C_NONE) {
       int32_t id = c->fill;
       auto search_univ = universe_map.find(id);
       auto search_lat = lattice_map.find(id);

src/geometry_header.F90

@@ -4,7 +4,7 @@ module geometry_header
 
   use algorithm,       only: find
   use constants,       only: HALF, TWO, THREE, INFINITY, K_BOLTZMANN, &
-                             MATERIAL_VOID, NONE
+                             MATERIAL_VOID
   use dict_header,     only: DictCharInt, DictIntInt
   use hdf5_interface,  only: HID_T
   use material_header, only: Material, materials, material_dict, n_materials
@@ -462,10 +462,12 @@ subroutine get_temperatures(nuc_temps, sab_temps)
     if (present(sab_temps)) allocate(sab_temps(n_sab_tables))
 
     do i = 1, size(cells)
+      ! Skip non-material cells.
+      if (cells(i) % fill() /= C_NONE) cycle
+
       do j = 1, cells(i) % material_size()
-        ! Skip any non-material cells and void materials
-        if (cells(i) % material(j) == NONE .or. &
-             cells(i) % material(j) == MATERIAL_VOID) cycle
+        ! Skip void materials
+        if (cells(i) % material(j) == MATERIAL_VOID) cycle
 
         ! Get temperature of cell (rounding to nearest integer)
         if (size(cells(i) % sqrtkT) > 1) then

src/input_xml.F90

@@ -1215,7 +1215,7 @@ subroutine read_geometry_xml()
         n = node_word_count(node_cell, "temperature")
         if (n > 0) then
           ! Make sure this is a "normal" cell.
-          if (c % material(1) == NONE) call fatal_error("Cell " &
+          if (c % fill() /= C_NONE) call fatal_error("Cell " &
                // trim(to_str(c % id())) // " was specified with a temperature &
                &but no material. Temperature specification is only valid for &
                &cells filled with a material.")
@@ -3814,7 +3814,7 @@ subroutine assign_temperatures(material_temps)
 
     do i = 1, n_cells
       ! Ignore non-normal cells and cells with defined temperature.
-      if (cells(i) % material(1) == NONE) cycle
+      if (cells(i) % fill() /= C_NONE) cycle
       if (cells(i) % sqrtkT(1) >= ZERO) cycle
 
       ! Set the number of temperatures equal to the number of materials.

src/lattice.cpp

@@ -29,7 +29,7 @@ std::unordered_map<int32_t, int32_t> lattice_map;
 Lattice::Lattice(pugi::xml_node lat_node)
 {
   if (check_for_node(lat_node, "id")) {
-    id = stoi(get_node_value(lat_node, "id"));
+    id = std::stoi(get_node_value(lat_node, "id"));
   } else {
     fatal_error("Must specify id of lattice in geometry XML file.");
   }
@@ -39,7 +39,7 @@ Lattice::Lattice(pugi::xml_node lat_node)
   }
 
   if (check_for_node(lat_node, "outer")) {
-    outer = stoi(get_node_value(lat_node, "outer"));
+    outer = std::stoi(get_node_value(lat_node, "outer"));
   }
 }
 
@@ -141,14 +141,14 @@ RectLattice::RectLattice(pugi::xml_node lat_node)
   std::string dimension_str {get_node_value(lat_node, "dimension")};
   std::vector<std::string> dimension_words {split(dimension_str)};
   if (dimension_words.size() == 2) {
-    n_cells[0] = stoi(dimension_words[0]);
-    n_cells[1] = stoi(dimension_words[1]);
+    n_cells[0] = std::stoi(dimension_words[0]);
+    n_cells[1] = std::stoi(dimension_words[1]);
     n_cells[2] = 1;
     is_3d = false;
   } else if (dimension_words.size() == 3) {
-    n_cells[0] = stoi(dimension_words[0]);
-    n_cells[1] = stoi(dimension_words[1]);
-    n_cells[2] = stoi(dimension_words[2]);
+    n_cells[0] = std::stoi(dimension_words[0]);
+    n_cells[1] = std::stoi(dimension_words[1]);
+    n_cells[2] = std::stoi(dimension_words[2]);
     is_3d = true;
   } else {
     fatal_error("Rectangular lattice must be two or three dimensions.");
@@ -195,7 +195,7 @@ RectLattice::RectLattice(pugi::xml_node lat_node)
       for (int ix = 0; ix < nx; ix++) {
         int indx1 = nx*ny*iz + nx*(ny-iy-1) + ix;
         int indx2 = nx*ny*iz + nx*iy + ix;
-        universes[indx1] = stoi(univ_words[indx2]);
+        universes[indx1] = std::stoi(univ_words[indx2]);
       }
     }
   }
@@ -400,9 +400,9 @@ HexLattice::HexLattice(pugi::xml_node lat_node)
   : Lattice {lat_node}
 {
   // Read the number of lattice cells in each dimension.
-  n_rings = stoi(get_node_value(lat_node, "n_rings"));
+  n_rings = std::stoi(get_node_value(lat_node, "n_rings"));
   if (check_for_node(lat_node, "n_axial")) {
-    n_axial = stoi(get_node_value(lat_node, "n_axial"));
+    n_axial = std::stoi(get_node_value(lat_node, "n_axial"));
     is_3d = true;
   } else {
     n_axial = 1;
@@ -476,7 +476,7 @@ HexLattice::HexLattice(pugi::xml_node lat_node)
         int indx = (2*n_rings-1)*(2*n_rings-1) * m
                     + (2*n_rings-1) * (i_a+n_rings-1)
                     + (i_x+n_rings-1);
-        universes[indx] = stoi(univ_words[input_index]);
+        universes[indx] = std::stoi(univ_words[input_index]);
         input_index++;
         // Walk the index to the right neighbor (which is not adjacent).
         i_x += 2;
@@ -505,7 +505,7 @@ HexLattice::HexLattice(pugi::xml_node lat_node)
         int indx = (2*n_rings-1)*(2*n_rings-1) * m
                     + (2*n_rings-1) * (i_a+n_rings-1)
                     + (i_x+n_rings-1);
-        universes[indx] = stoi(univ_words[input_index]);
+        universes[indx] = std::stoi(univ_words[input_index]);
         input_index++;
         // Walk the index to the right neighbor (which is not adjacent).
         i_x += 2;
@@ -528,7 +528,7 @@ HexLattice::HexLattice(pugi::xml_node lat_node)
         int indx = (2*n_rings-1)*(2*n_rings-1) * m
                     + (2*n_rings-1) * (i_a+n_rings-1)
                     + (i_x+n_rings-1);
-        universes[indx] = stoi(univ_words[input_index]);
+        universes[indx] = std::stoi(univ_words[input_index]);
         input_index++;
         // Walk the index to the right neighbor (which is not adjacent).
         i_x += 2;

src/material.cpp

@@ -6,9 +6,6 @@
 #include "error.h"
 #include "xml_interface.h"
 
-//TODO: remove this include
-#include <iostream>
-
 
 namespace openmc {
 
@@ -26,7 +23,7 @@ std::unordered_map<int32_t, int32_t> material_map;
 Material::Material(pugi::xml_node material_node)
 {
   if (check_for_node(material_node, "id")) {
-    id = stoi(get_node_value(material_node, "id"));
+    id = std::stoi(get_node_value(material_node, "id"));
   } else {
     fatal_error("Must specify id of material in materials XML file.");
   }
@@ -40,7 +37,7 @@ extern "C" void
 read_materials(pugi::xml_node* node)
 {
   // Loop over XML material elements and populate the array.
-  for (pugi::xml_node material_node: node->children("material")) {
+  for (pugi::xml_node material_node : node->children("material")) {
     global_materials.push_back(new Material(material_node));
   }
   global_materials.shrink_to_fit();

src/mgxs_data.F90

@@ -151,11 +151,13 @@ subroutine get_mat_kTs(kTs)
     allocate(kTs(size(materials)))
 
     do i = 1, size(cells)
+      ! Skip non-material cells
+      if (cells(i) % fill() /= C_NONE) cycle
+
       do j = 1, cells(i) % material_size()
 
-        ! Skip any non-material cells and void materials
-        if (cells(i) % material(j) == NONE .or. &
-             cells(i) % material(j) == MATERIAL_VOID) cycle
+        ! Skip void materials
+        if (cells(i) % material(j) == MATERIAL_VOID) cycle
 
         ! Get temperature of cell (rounding to nearest integer)
         if (size(cells(i) % sqrtkT) > 1) then

src/surface.cpp

@@ -141,7 +141,7 @@ void read_coeffs(pugi::xml_node surf_node, int surf_id, double &c1, double &c2,
 Surface::Surface(pugi::xml_node surf_node)
 {
   if (check_for_node(surf_node, "id")) {
-    id = stoi(get_node_value(surf_node, "id"));
+    id = std::stoi(get_node_value(surf_node, "id"));
   } else {
     fatal_error("Must specify id of surface in geometry XML file.");
   }
@@ -247,7 +247,7 @@ PeriodicSurface::PeriodicSurface(pugi::xml_node surf_node)
   : Surface {surf_node}
 {
   if (check_for_node(surf_node, "periodic_surface_id")) {
-    i_periodic = stoi(get_node_value(surf_node, "periodic_surface_id"));
+    i_periodic = std::stoi(get_node_value(surf_node, "periodic_surface_id"));
   }
 }
 

src/xml_interface.h

@@ -17,13 +17,14 @@ check_for_node(pugi::xml_node node, const char *name)
 }
 
 std::string get_node_value(pugi::xml_node node, const char *name,
-                           bool lowercase=true, bool strip=true);
+                           bool lowercase=false, bool strip=false);
 
 template <typename T>
-std::vector<T> get_node_array(pugi::xml_node node, const char* name)
+std::vector<T> get_node_array(pugi::xml_node node, const char* name,
+                              bool lowercase=false)
 {
   // Get value of node attribute/child
-  std::string s {get_node_value(node, name)};
+  std::string s {get_node_value(node, name, lowercase)};
 
   // Read values one by one into vector
   std::stringstream iss {s};