improve exception handling

adds additional classes splitting into expected and unexpected errors

resolves #22
partially resolves #25

src/baseconfiguration.cpp

@@ -25,15 +25,16 @@ const std::string ConfigurationBase::k_extension_token = "%ext%";
 const std::string ConfigurationBase::k_outer_token = "%s%";
 const std::string ConfigurationBase::k_inner_token = "%i%";
 
-const config_map ConfigurationBase::arg_options = {
-    {"registered", "registered.xdmf"}, {"map", "map.xdmf"}, {"lambda", "auto"}, {"mask", ""},
-    {"registered_h5_path", "/registered"}, {"map_h5_path", "/map"}, {"lambda_mult", "1.0"},
+const config_map ConfigurationBase::arg_options = {{"fixed", ""}, {"moved", ""},
+    {"nodespacing", ""}, {"registered", "registered.xdmf"}, {"map", "map.xdmf"},
+    {"lambda", "auto"}, {"mask", ""}, {"registered_h5_path", "/registered"},
+    {"map_h5_path", "/map"}, {"lambda_mult", "1.0"},
     {"intermediate_template", "%name%-intermediate-%s%-%i%%ext%"},
     {"intermediate_map_template", "%name%-intermediate-map-%s%-%i%%ext%"},
     {"intermediate_directory", "intermediates"}, {"max_iterations", "100"}};
 
 const config_map ConfigurationBase::bool_options = {
-  {"verbose", "false"}, {"with_memory", "true"}, {"save_intermediate_frames", "false"}};
+    {"verbose", "false"}, {"with_memory", "true"}, {"save_intermediate_frames", "false"}};
 
 const std::vector<std::string> ConfigurationBase::required_options = {
     "fixed", "moved", "nodespacing"};
@@ -50,12 +51,12 @@ void ConfigurationBase::validate_config()
   std::list<std::string> missing;
   for (auto &req_it : ConfigurationBase::required_options)
   {
-    if (config.find(req_it) == config.cend())
+    if (config.find(req_it) == config.cend() || config[req_it].empty())
     {
       missing.push_back(req_it);
     }
   }
-  if (missing.empty())
+  if (!missing.empty())
   {
     std::ostringstream errmsg;
     errmsg << "Missing required argument(s) \"";

src/baseconfiguration.hpp

@@ -30,6 +30,7 @@
 namespace bf = boost::filesystem;
 
 #include "types.hpp"
+#include "exceptions.hpp"
 
 using config_map = std::map<std::string, std::string>;
 
@@ -39,29 +40,57 @@ class ConfigurationBase {
   template <typename T>
   typename std::enable_if<std::is_same<T, bool>::value, T>::type grab(const std::string key) const
   {
-    T val;
-    std::istringstream(config.at(key)) >> std::boolalpha >> val;
-    return val;
+    try
+    {
+      T val;
+      std::istringstream(config.at(key)) >> std::boolalpha >> val;
+      return val;
+    }
+    catch (std::out_of_range &err)
+    {
+      throw InternalError("Attempt to access non-existent configuration option", __FILE__, __LINE__);
+    }
   }
 
   template <typename T>
   typename std::enable_if<std::is_integral<T>::value && !std::is_same<T, bool>::value, T>::type
   grab(const std::string key) const
   {
-    return static_cast<T>(std::stoll(config.at(key)));
+    try
+    {
+      return static_cast<T>(std::stoll(config.at(key)));
+    }
+    catch (std::out_of_range &err)
+    {
+      throw InternalError("Attempt to access non-existent configuration option", __FILE__, __LINE__);
+    }
   }
 
   template <typename T>
   typename std::enable_if<std::is_floating_point<T>::value && !std::is_same<T, bool>::value, T>::type
   grab(const std::string key) const
   {
-    return static_cast<T>(std::stold(config.at(key)));
-  }
+    try
+    {
+      return static_cast<T>(std::stold(config.at(key)));
+    }
+    catch (std::out_of_range &err)
+    {
+      throw InternalError("Attempt to access non-existent configuration option", __FILE__, __LINE__);
+    }
+}
 
   template <typename T>
   typename std::enable_if<std::is_same<T, std::string>::value, T>::type grab(std::string key) const
   {
-    return config.at(key);
+    try
+    {
+      return config.at(key);
+    }
+    catch (std::out_of_range &err)
+    {
+      throw InternalError("Attempt to access non-existent configuration option", __FILE__, __LINE__);
+    }
   }
 
   static std::string get_invocation_name(const std::string& argzero);

src/baseloader.cpp

@@ -33,7 +33,7 @@ bool BaseLoader::register_loader(const std::string &name, loader_creator loader)
   }
   std::ostringstream err;
   err << "Loader named " << name << " already registered.";
-  throw std::runtime_error(err.str());
+  throw InternalError(err.str(), __FILE__, __LINE__);
 }
 
 BaseLoader_unique BaseLoader::find_loader(const std::string &path, MPI_Comm comm)
@@ -53,5 +53,5 @@ BaseLoader_unique BaseLoader::find_loader(const std::string &path, MPI_Comm comm
   }
   std::ostringstream err;
   err << "Failed to find suitable loader for \"" << path << "\".";
-  throw std::runtime_error(err.str());
+  throw InvalidLoaderError(err.str());
 }

src/basewriter.cpp

@@ -54,7 +54,7 @@ std::string BaseWriter::find_writer_for_filename(const std::string &filename)
   }
   std::ostringstream err;
   err << "No suitable writer available for extension \"" << extension << "\".";
-  throw std::runtime_error(err.str());
+  throw BadConfigurationError(err.str());
 }
 
 BaseWriter_unique BaseWriter::get_writer_by_name(
@@ -68,7 +68,7 @@ BaseWriter_unique BaseWriter::get_writer_by_name(
   }
   std::ostringstream err;
   err << "No registered writer named \"" << name << "\".";
-  throw std::runtime_error(err.str());
+  throw BadConfigurationError(err.str());
 }
 
 BaseWriter_unique BaseWriter::get_writer_for_filename(const std::string &filename, MPI_Comm comm)
@@ -78,12 +78,12 @@ BaseWriter_unique BaseWriter::get_writer_for_filename(const std::string &filenam
     return BaseWriter::get_writer_by_name(
         BaseWriter::find_writer_for_filename(filename), filename, comm);
   }
-  catch (const std::runtime_error &errstr)
+  catch (const pFIREExpectedError &errstr)
   {
     std::ostringstream err;
     err << "No suitable writer available for file \"" << filename << "\"."
         << "\n(" << errstr.what() << ")\n";
-    throw std::runtime_error(err.str());
+    throw BadConfigurationError(err.str());
   }
 }
 

src/basewriter.hpp

@@ -25,6 +25,7 @@
 #include <mpi.h>
 
 #include "types.hpp"
+#include "exceptions.hpp"
 
 class BaseWriter {
 public:
@@ -107,7 +108,7 @@ BaseWriter::register_writer()
   }
   std::ostringstream err;
   err << "Writer named " << WriterClass::writer_name << " already registered.";
-  throw std::runtime_error(err.str());
+  throw InternalError(err.str(), __FILE__, __LINE__);
 }
 
 #endif // BASEWRITER_HPP

src/basis.cpp

@@ -18,6 +18,7 @@
 #include <petscmat.h>
 
 #include "debug.hpp"
+#include "exceptions.hpp"
 #include "petsc_debug.hpp"
 #include "indexing.hpp"
 #include "iterator_routines.hpp"
@@ -142,12 +143,12 @@ Mat_unique build_warp_matrix(
   integer mat_size = std::accumulate(img_shape.begin(), img_shape.end(), 1, std::multiplies<>());
   if (img_shape.size() < ndim)
   {
-    throw std::runtime_error("image dimensions must match ndim");
+    throw InternalError("image dimensions must match ndim", __FILE__, __LINE__);
   }
 
   if (displacements.size() < ndim)
   {
-    throw std::runtime_error("must have displacement vector for each image dimension");
+    throw InternalError("must have displacement vector for each image dimension", __FILE__, __LINE__);
   }
 
   intvector img_shape_trunc(ndim, 0);

src/dcmloader.cpp

@@ -58,19 +58,19 @@ DCMLoader::DCMLoader(const std::string &path, MPI_Comm comm)
   status = dataset->findAndGetLongInt(DcmTagKey(TG_IMG, TE_ROWS), tmp);
   if (status.bad())
   {
-    throw std::runtime_error("Failed to read image shape data");
+    throw InvalidLoaderError("Failed to read image shape data");
   }
   this->_shape[0] = tmp;
   status = dataset->findAndGetLongInt(DcmTagKey(TG_IMG, TE_COLS), tmp);
   if (status.bad())
   {
-    throw std::runtime_error("Failed to read image shape data");
+    throw InvalidLoaderError("Failed to read image shape data");
   }
   this->_shape[1] = tmp;
   status = dataset->findAndGetLongInt(DcmTagKey(TG_IMG, TE_FRAMES), tmp);
   if (status.bad())
   {
-    throw std::runtime_error("Failed to read image shape data");
+    throw InvalidLoaderError("Failed to read image shape data");
   }
   this->_shape[2] = tmp;
 }
@@ -123,7 +123,7 @@ void DCMLoader::copy_scaled_chunk(
         }
         default:
         {
-          throw std::runtime_error("Unhandled image bit depth.");
+          throw InvalidLoaderError("Unhandled image bit depth.");
           break;
         }
       }

src/elastic.cpp

@@ -129,7 +129,7 @@ void Elastic::innerloop(integer outer_count)
     innerstep(inum, recalculate_lambda);
     recalculate_lambda = false;
 
-    if (configuration.grab<bool>("save_intermediate_frames"))
+    if (configuration.grab<bool>("save_intermediate_flames"))
     {
       save_debug_frame(outer_count, inum);
       save_debug_map(outer_count, inum);
@@ -149,7 +149,7 @@ void Elastic::innerloop(integer outer_count)
     PetscPrintf(m_comm, "Average displacement: %.2f\n", aavg);
     floating curr_mi = m_p_registered->mutual_information(m_fixed);
     PetscPrintf(m_comm, "Mutual information: %f\n", curr_mi);
-    if (aavg < m_convergence_thres || curr_mi <= prev_mi)
+    if (aavg < m_convergence_thres)// || curr_mi <= prev_mi)
     {
       PetscPrintf(m_comm, "Generation %i converged after %i iterations.\n\n", outer_count, inum);
       break;
@@ -326,8 +326,9 @@ void Elastic::save_debug_map(integer outer_count, integer inner_count)
     if (!bf::exists(intermediates_path))
     {
       std::ostringstream errss;
-      errss << "Intermediate frame output path " << intermediates_path << " does not exist.";
-      throw std::runtime_error(errss.str());
+      errss << "Intermediate frame output path " << intermediates_path << " does not exist, but "
+            << "should have already been created.";
+      throw InternalError(errss.str(), __FILE__, __LINE__);
     }
     output_path = intermediates_path;
   }

src/exceptions.hpp

@@ -23,66 +23,114 @@
 void abort_with_unhandled_error();
 void sigterm_handler(int signal);
 
-class InvalidLoaderError: public std::runtime_error {
+class pFIREBaseError: public std::runtime_error {
 public:
-  InvalidLoaderError(const std::string& filepath) : std::runtime_error(build_errstring(filepath))
+  pFIREBaseError(const std::string &msg, const std::string &file, size_t line_num)
+    : std::runtime_error(msg), _where(build_wherestring(file, line_num))
   {
   }
 
+  const char *where() { return _where.c_str(); }
+
 protected:
-  static std::string build_errstring(const std::string& path)
+  std::string _where;
+
+  static std::string build_wherestring(const std::string &file, size_t line_num)
   {
     std::ostringstream errss;
-    errss << "Failed to read data from " << path << ", wrong loader or file corrupt.";
+    errss << file << ":" << line_num;
     return errss.str();
   }
+
+  static std::string build_errstring(const std::string &what)
+  {
+    std::ostringstream errss;
+    errss << what;
+    return errss.str();
+  }
+
 };
 
-class FileNotFoundError: public std::runtime_error {
+class pFIREExpectedError: public pFIREBaseError {
 public:
-  FileNotFoundError(const std::string& filepath) : std::runtime_error(build_errstring(filepath)) {}
+  pFIREExpectedError(const std::string &msg, const std::string& file, size_t line_num)
+    : pFIREBaseError(msg, file, line_num)
+  {}
+
+};
+
+class InvalidLoaderError: public pFIREExpectedError {
+public:
+  InvalidLoaderError(const std::string &path, std::string file = "unknown", size_t line_num = 0)
+    : pFIREExpectedError(build_errstring(path), file, line_num)
+  {
+  }
 
 protected:
-  static std::string build_errstring(const std::string& path)
+  static std::string build_errstring(const std::string &path)
   {
     std::ostringstream errss;
-    errss << "Failed to open " << path << ", wrong permissions or file does not exist.";
+    errss << "Failed to read data from " << path << ", wrong loader or file corrupt.";
     return errss.str();
   }
 };
 
-class InternalError: public std::runtime_error {
+class FileNotFoundError: public pFIREExpectedError {
 public:
-  InternalError(const std::string& what, std::string file = "unknown", integer line = 0)
-    : std::runtime_error(build_errstring(what, file, line))
+  FileNotFoundError(const std::string &path, std::string file = "unknown", size_t line_num = 0)
+    : pFIREExpectedError(build_errstring(path), file, line_num)
   {
   }
 
 protected:
-  static std::string build_errstring(
-      const std::string& what, std::string file, integer line)
+  static std::string build_errstring(const std::string &path)
   {
     std::ostringstream errss;
-    errss << "Internal error at " << file << ":" << line << " \"" << what << "\"";
+    errss << "Failed to open " << path << ", wrong permissions or file does not exist.";
     return errss.str();
   }
 };
 
-class BadConfigurationError: public std::runtime_error {
+class InternalError: public pFIREBaseError {
 public:
-  BadConfigurationError(const std::string& what, std::string file = "unknown", integer line = 0)
-    : std::runtime_error(build_errstring(what, file, line))
+  InternalError(const std::string &what, std::string file = "unknown", size_t line = 0)
+    : pFIREBaseError(build_errstring(what, file, line), file, line)
   {
   }
 
 protected:
-  static std::string build_errstring(
-      const std::string& what, std::string file, integer line)
+  std::string build_errstring(const std::string &what, const std::string &file, size_t line)
   {
     std::ostringstream errss;
     errss << "Internal error at " << file << ":" << line << " \"" << what << "\"";
     return errss.str();
   }
 };
 
+class BadConfigurationError: public pFIREExpectedError {
+public:
+  BadConfigurationError(const std::string &msg, std::string file = "unknown", size_t line = 0)
+    : pFIREExpectedError(build_errstring(msg), file, line)
+  {
+  }
+
+};
+
+class InvalidWriterError: public pFIREExpectedError {
+public:
+  InvalidWriterError(const std::string &msg, std::string file = "unknown", size_t line = 0)
+    : pFIREExpectedError(build_errstring(msg), file, line)
+  {
+  }
+};
+
+
+class WriterError: public pFIREExpectedError {
+public:
+  WriterError(const std::string &msg, std::string file = "unknown", size_t line = 0)
+    : pFIREExpectedError(build_errstring(msg), file, line)
+  {
+  }
+};
+
 #endif // EXCEPTIONS_HPP