Addressing a number of miscellaneous changes from #660

.gitignore

@@ -5,6 +5,7 @@
 #*
 # vscode settings
 .vscode
+*.code-workspace
 
 # Ignore built documentation
 gh-build

misc/tests/test_uwuw_preproc_model.py

@@ -28,9 +28,8 @@ def test_model_material():
     # group_list is the final list of group names found on the CAD h5m file
     for k in pseudo_list:
         if ("mat:" in k):
-            index = k.index('\n')
-            k = k[0:index]
-            group_list.append(k.rstrip("\n"))
+            idx = k.index('\n')
+            group_list.append(k[0:idx].rstrip("\n"))
     # tag_list is the list of group names obtained by running get_tag_values
     # function
     tag_list = gtag.get_tag_values(filename, output_filename)

news/PR-0680.rst

@@ -0,0 +1,18 @@
+**Added:** None
+
+**Changed:** 
+* a few small fixes prompted by PullRequest review #660
+* only changed files in 
+    * `uwuw`
+    * `tally`
+    * `overlap_check`
+    * `build_obb`
+    * `misc/tests`
+
+**Deprecated:** None
+
+**Removed:** None
+
+**Fixed:** None
+
+**Security:** None

src/build_obb/build_obb.cpp

@@ -25,7 +25,12 @@ int main(int argc, char* argv[]) {
   // sets the output filename if none specified
   if (out_file == "") {
     int pos = dag_file.find(".h5m");
-    out_file = dag_file.substr(0, pos) + "_obb.h5m";
+    if (pos != -1) {
+      out_file = dag_file.substr(0, pos);
+    } else {
+      out_file = dag_file;
+    }
+    out_file = out_file + "_obb.h5m";
     std::cout << "Setting default outfile to be " << out_file << std::endl;
   }
 

src/overlap_check/ProgressBar.cpp

@@ -2,6 +2,7 @@
 
 #include "ProgressBar.hpp"
 
+#include <string>
 #include <sstream>
 #include <iostream>
 #include <iomanip>
@@ -12,13 +13,10 @@
 
 #define BAR_WIDTH 72
 
-ProgressBar::ProgressBar() {
-  // initialize bar
-  set_value(0.0);
-}
-
 ProgressBar::~ProgressBar() {
-  std::cout << "\n";
+  if (need_final_newline) {
+    std::cout << std::endl;
+  }
 }
 
 bool ProgressBar::is_terminal() {
@@ -62,7 +60,9 @@ void ProgressBar::set_value(double val) {
 
   // write the bar to screen
   std::cout << '\r' << bar.str() << std::flush;
+  need_final_newline = true;
   if (val >= 100.0) {
     std::cout << "\n";
+    need_final_newline = false;
   }
 }

src/overlap_check/ProgressBar.hpp

@@ -2,13 +2,14 @@
 #ifndef DAGMC_PROGRESSBAR_H
 #define DAGMC_PROGRESSBAR_H
 
-#include <string>
-
 class ProgressBar {
 
  public:
   // constructor
-  ProgressBar();
+  ProgressBar() {
+    // initialize bar
+    set_value(0.0);
+  };
 
   // destructor
   ~ProgressBar();
@@ -19,6 +20,7 @@ class ProgressBar {
 
  private:
   int current {0};
+  bool need_final_newline {false};
 };
 
 #endif // HEADER GUARD

src/overlap_check/app/overlap_check.cpp

@@ -19,7 +19,9 @@ int main(int argc, char* argv[]) {
 
   std::string filename;
   int points_per_tri_edge {0};
+#ifdef _OPENMP
   int n_threads {0};
+#endif
 
   po.addRequiredArg<std::string>("dag_file", "Path to DAGMC file to check", &filename);
 

src/overlap_check/overlap.cpp

@@ -104,11 +104,11 @@ check_instance_for_overlaps(std::shared_ptr<Interface> MBI,
   #pragma omp parallel shared(overlap_map, num_checked)
   {
     #pragma omp for schedule(auto)
-    for (int i = 0; i < all_verts.size(); i++) {
+    for (size_t i = 0; i < all_verts.size(); i++) {
 
       EntityHandle vert = all_verts[i];
       CartVect loc;
-      rval = MBI->get_coords(&vert, 1, loc.array());
+      MBI->get_coords(&vert, 1, loc.array());
 
       rval = check_location_for_overlap(GQT, all_vols, loc, dir, overlap_map);
       MB_CHK_SET_ERR_CONT(rval, "Failed to check location " << loc << " for an overlap");
@@ -129,7 +129,7 @@ check_instance_for_overlaps(std::shared_ptr<Interface> MBI,
     // (curve edges are likely in here too,
     //  but it isn't hurting anything to check more locations)
     #pragma omp for schedule(auto)
-    for (int i = 0; i < all_edges.size() ; i++) {
+    for (size_t i = 0; i < all_edges.size() ; i++) {
 
       EntityHandle edge = all_edges[i];
       Range edge_verts;

src/overlap_check/test/overlap_check_test.cpp

@@ -27,7 +27,7 @@ void OverlapTest::TearDown() {
 };
 
 class OverlappingVolumesTest : public OverlapTest {
-  virtual void SetFilename() { filename = "overlap.h5m"; }
+  virtual void SetFilename() override { filename = "overlap.h5m"; }
 };
 
 TEST_F(OverlappingVolumesTest, test1) {
@@ -43,7 +43,7 @@ TEST_F(OverlappingVolumesTest, test1) {
 }
 
 class NonOverlappingVolumesTest : public OverlapTest {
-  virtual void SetFilename() { filename = "no_overlap.h5m"; }
+  virtual void SetFilename() override { filename = "no_overlap.h5m"; }
 };
 
 TEST_F(NonOverlappingVolumesTest, test2) {
@@ -62,7 +62,7 @@ TEST_F(NonOverlappingVolumesTest, test2) {
 }
 
 class NonOverlappingImprintedVolumesTest : public OverlapTest {
-  virtual void SetFilename() { filename = "no_overlap_imp.h5m"; }
+  virtual void SetFilename() override { filename = "no_overlap_imp.h5m"; }
 };
 
 TEST_F(NonOverlappingImprintedVolumesTest, test3) {
@@ -81,7 +81,7 @@ TEST_F(NonOverlappingImprintedVolumesTest, test3) {
 }
 
 class EnclosedVolumeTest : public OverlapTest {
-  virtual void SetFilename() { filename = "enclosed.h5m"; }
+  virtual void SetFilename() override { filename = "enclosed.h5m"; }
 };
 
 TEST_F(EnclosedVolumeTest, test1) {

src/overlap_check/test/overlap_check_test.hpp

@@ -5,8 +5,8 @@ using namespace moab;
 
 class OverlapTest : public::testing::Test {
  protected:
-  virtual void SetUp();
-  virtual void TearDown();
+  virtual void SetUp() override;
+  virtual void TearDown() override;
   virtual void SetFilename() {};
 
   std::string filename;

src/tally/KDEMeshTally.cpp

@@ -188,8 +188,8 @@ void KDEMeshTally::write_data(double num_histories) {
     if (data->has_total_energy_bin())
       num_ebins--;
 
-    double tally_vect[num_ebins];
-    double error_vect[num_ebins];
+    std::vector<double> tally_vect(num_ebins);
+    std::vector<double> error_vect(num_ebins);
 
     for (unsigned int j = 0; j < num_ebins; ++ j) {
       std::pair <double, double> tally_data = data->get_data(point_index, j);
@@ -210,9 +210,9 @@ void KDEMeshTally::write_data(double num_histories) {
       error_vect[j] = rel_error;
     }
     // set tally and error tag values for this entity
-    rval = mbi->tag_set_data(tally_tag, &point, 1, tally_vect);
+    rval = mbi->tag_set_data(tally_tag, &point, 1, tally_vect.data());
     MB_CHK_SET_ERR_RET(rval, "Failed to set the tally_tag data");
-    rval = mbi->tag_set_data(error_tag, &point, 1, error_vect);
+    rval = mbi->tag_set_data(error_tag, &point, 1, error_vect.data());
     MB_CHK_SET_ERR_RET(rval, "Failed to set the error_tag data");
   }
 

src/tally/TrackLengthMeshTally.cpp

@@ -209,7 +209,7 @@ void TrackLengthMeshTally::compute_score(const TallyEvent& event) {
   sort_intersection_data(intersections, triangles);
 
   // compute the tracklengths
-  compute_tracklengths(event, ebin, weight, intersections, triangles);
+  compute_tracklengths(event, ebin, weight, intersections);
 
   return;
 }
@@ -261,8 +261,8 @@ void TrackLengthMeshTally::write_data(double num_histories) {
     if (data->has_total_energy_bin())
       num_ebins--;
 
-    double tally_vect[num_ebins];
-    double error_vect[num_ebins];
+    std::vector<double> tally_vect(num_ebins);
+    std::vector<double> error_vect(num_ebins);
 
     for (unsigned j = 0; j < num_ebins ; ++j) {
       std::pair <double, double> tally_data = data->get_data(tet_index, j);
@@ -282,9 +282,9 @@ void TrackLengthMeshTally::write_data(double num_histories) {
       error_vect[j] = rel_err;
     }
 
-    rval = mb->tag_set_data(tally_tag, &t, 1, tally_vect);
+    rval = mb->tag_set_data(tally_tag, &t, 1, tally_vect.data());
     MB_CHK_SET_ERR_RET(rval, "Failed to set tally_tag " + std::to_string(rval) + " " + std::to_string(t));
-    rval = mb->tag_set_data(error_tag, &t, 1, error_vect);
+    rval = mb->tag_set_data(error_tag, &t, 1, error_vect.data());
     MB_CHK_SET_ERR_RET(rval, "Failed to set error_tag " + std::to_string(rval) + " " + std::to_string(t));
 
 
@@ -630,8 +630,7 @@ void TrackLengthMeshTally::sort_intersection_data(std::vector<double>& intersect
 // function to compute the track lengths
 void TrackLengthMeshTally::compute_tracklengths(const TallyEvent& event,
                                                 unsigned int ebin, double weight,
-                                                const std::vector<double>& intersections,
-                                                const std::vector<EntityHandle>& triangles) {
+                                                const std::vector<double>& intersections) {
   double track_length; // track_length to add to the tet
   CartVect hit_p; //position on the triangular face of the hit
   std::vector<CartVect> hit_point; // array of all hit points

src/tally/TrackLengthMeshTally.hpp

@@ -239,8 +239,7 @@ class TrackLengthMeshTally : public MeshTally {
    */
   void compute_tracklengths(const TallyEvent& event,
                             unsigned int ebin, double weight,
-                            const std::vector<double>& intersections,
-                            const std::vector<EntityHandle>& triangles);
+                            const std::vector<double>& intersections);
 
 };
 

src/tally/tests/test_KDEKernel.cpp

@@ -24,9 +24,9 @@ class MockEpanechnikovKernel : public KDEKernel {
   }
 
   // not implemented
-  std::string get_kernel_name() const {}
-  int get_order() const {}
-  int get_min_quadrature(unsigned int i) const {}
+  std::string get_kernel_name() const { return std::string(); }
+  int get_order() const { return 0; }
+  int get_min_quadrature(unsigned int i) const { return 0; }
 
   // integrates the ith moment function
   double integrate_moment(double a, double b, unsigned int i) const {

src/uwuw/name_concatenator.hpp

@@ -5,10 +5,10 @@
 class name_concatenator {
  public:
   /// constructor
-  name_concatenator();
+  name_concatenator() {};
 
   /// destructor
-  ~name_concatenator();
+  ~name_concatenator() {};
 
   /// returns the a unique name
   std::string make_name_8bytes(std::string name);

src/uwuw/uwuw.cpp

@@ -65,7 +65,7 @@ std::string UWUW::get_full_filepath(std::string filename) {
 }
 
 // see if file exists
-bool UWUW::check_file_exists(std::string filename) {
+bool UWUW::check_file_exists(const std::string& filename) {
   // from http://stackoverflow.com/questions/12774207/
   // fastest-way-to-check-if-a-file-exist-using-standard-c-c11-c
   std::ifstream infile(filename.c_str());
@@ -151,7 +151,7 @@ int UWUW::get_length_of_table(std::string filename, std::string datapath) {
   hid_t arr_space = H5Dget_space(ds);
 
   hsize_t arr_dims[1];
-  int arr_ndim = H5Sget_simple_extent_dims(arr_space, arr_dims, NULL);
+  H5Sget_simple_extent_dims(arr_space, arr_dims, NULL);
 
   status = H5Eclear(H5E_DEFAULT);
 

src/uwuw/uwuw.hpp

@@ -103,7 +103,7 @@ class UWUW {
   // turns the filename string into the full file path
   std::string get_full_filepath(std::string filename);
   // make sure the file, filename exists
-  bool check_file_exists(std::string filename);
+  bool check_file_exists(const std::string& filename);
 
   /**
    * \brief loads the pyne materials in map of name vs Material

src/uwuw/uwuw_preprocessor.cpp

@@ -96,11 +96,10 @@ pyne::Material uwuw_preprocessor::create_new_material(pyne::Material material, s
 
   std::string material_name;
   // make a new name
+  material_name = "mat:" + material.metadata["name"].asString();
   if (density != "") {
-    material_name = "mat:" + material.metadata["name"].asString() + "/rho:" + density;
+    material_name = material_name + "/rho:" + density;
     new_mat.density = atof(density.c_str());
-  } else {
-    material_name = "mat:" + material.metadata["name"].asString();
   }
 
   if (verbose) {
@@ -178,7 +177,7 @@ void uwuw_preprocessor::process_tallies() {
   std::vector<std::string>::iterator it;
 
   // first volumes
-  for (int i = 1 ; i <= DAG->num_entities(3); i++) {
+  for (size_t i = 1 ; i <= DAG->num_entities(3); i++) {
     // get the tally properties
     std::string prop = dmd->get_volume_property("tally", i, true);
     std::vector<std::string> tally_props = dmd->unpack_string(prop, "|");
@@ -191,7 +190,7 @@ void uwuw_preprocessor::process_tallies() {
     }
   }
   // now surfaces
-  for (int i = 1 ; i <= DAG->num_entities(2); i++) {
+  for (size_t i = 1 ; i <= DAG->num_entities(2); i++) {
     // get the tally properties
     std::string prop = dmd->get_surface_property("tally", i, true);
     std::vector<std::string> tally_props = dmd->unpack_string(prop, "|");
@@ -245,7 +244,7 @@ void uwuw_preprocessor::check_material_props(std::vector<std::string> material_p
     if (verbose || fatal) {
       std::cout << "More than one material for volume with id " << cellid << std::endl;
       std::cout << cellid << " has the following material assignments" << std::endl;
-      for (int j = 0 ; j < material_props.size() ; j++) {
+      for (size_t j = 0 ; j < material_props.size() ; j++) {
         std::cout << material_props[j] << std::endl;
       }
       std::cout << "Please check your material assignments " << cellid << std::endl;
@@ -271,7 +270,7 @@ void uwuw_preprocessor::check_material_props(std::vector<std::string> material_p
     if (verbose || fatal) {
       std::cout << "More than one density specified for " << cellid << std::endl;
       std::cout << cellid << " has the following density assignments" << std::endl;
-      for (int j = 0 ; j < density_props.size() ; j++) {
+      for (size_t j = 0 ; j < density_props.size() ; j++) {
         std::cout << density_props[j] << std::endl;
       }
       std::cout << "Please check your density assignments " << cellid << std::endl;
@@ -303,10 +302,10 @@ void uwuw_preprocessor::print_summary() {
 }
 
 // used for printing & debugging only
-void uwuw_preprocessor::property_vector(std::vector<int> props) {
+void uwuw_preprocessor::property_vector(const std::vector<int>& props) {
   if (props.size() == 0)
     return;
-  for (int i = 0 ; i < props.size() ; i++) {
+  for (size_t i = 0 ; i < props.size() ; i++) {
     if (i == 0) {
       std::cout << "| " << props[i] << " ";
       continue;
@@ -436,14 +435,6 @@ void uwuw_preprocessor::check_tally_props(std::string particle_name,
 }
 
 
-// constructor
-name_concatenator::name_concatenator() {
-}
-
-// destructor
-name_concatenator::~name_concatenator() {
-}
-
 // make the fluka name from the string
 std::string name_concatenator::make_name_8bytes(std::string name) {
   // fluka name needs to be 8 chars long uppercase and unique