Clean up code 59 (#67)

SimVascular · Nov 7, 2023 · 3659f6d · 3659f6d
1 parent 92d98af
commit 3659f6d
Show file tree

Hide file tree

Showing 53 changed files with 655 additions and 880 deletions.
diff --git a/applications/svzerodcalibrator.cpp b/applications/svzerodcalibrator.cpp
@@ -33,8 +33,6 @@
  */
 #include "calibrate.h"
 
-// Setting scalar type to double
-
 int main(int argc, char* argv[]) {
   DEBUG_MSG("Starting svZeroDCalibrator");
 
@@ -43,21 +41,40 @@ int main(int argc, char* argv[]) {
     std::cout
         << "Usage: svzerodcalibrator path/to/config.json path/to/output.json"
         << std::endl;
-    exit(1);
+    return 1;
   }
 
   // Get input and output file names
-  std::string input_file = argv[1];
-  std::string output_file = argv[2];
+  std::string input_file_name = argv[1];
+  std::string output_file_name = argv[2];
 
   // Parse the configuration
   DEBUG_MSG("Parse configuration");
-  std::ifstream ifs(input_file);
-  const auto& config = nlohmann::json::parse(ifs);
+  std::ifstream input_file(input_file_name);
+
+  if (!input_file.is_open()) {
+    std::cerr << "[svzerodcalibrator] Error: The input file '" << input_file_name << "' cannot be opened." << std::endl;
+    return 1;
+  }
+
+  const auto& config = nlohmann::json::parse(input_file);
+  nlohmann::json output_config;
 
-  auto output_config = calibrate(config);
+  try {
+    output_config = calibrate(config);
+  } catch (const nlohmann::json::parse_error& e) {
+    std::cerr << "[svzerodcalibrator] Error: The input file '" << input_file_name
+    << "' does not have the parameters needed by the calibrate program." << std::endl;
+    return 1;
+  }
 
   // Write optimized simulation config
-  std::ofstream o(output_file);
-  o << std::setw(4) << output_config << std::endl;
+  std::ofstream out_file(output_file_name);
+
+  if (!out_file.is_open()) {
+    std::cerr << "[svzerodcalibrator] Error: The output file '" << output_file_name << "' cannot be opened." << std::endl;
+    return 1;
+  }
+
+  out_file << std::setw(4) << output_config << std::endl;
 }
diff --git a/applications/svzerodsolver.cpp b/applications/svzerodsolver.cpp
@@ -56,38 +56,56 @@ int main(int argc, char* argv[]) {
   DEBUG_MSG("Starting svZeroDSolver");
 
   // Get input and output file name
-  if (argc < 2 && argc > 3) {
-    std::runtime_error(
-        "Usage: svzerodsolver path/to/config.json "
-        "(optional:path/to/output.csv)");
+  if (argc < 2 || argc > 3) {
+    std::cout << "Usage: svzerodsolver path/to/config.json [path/to/output.csv]" << std::endl;;
+    return 1;
   }
 
-  std::string input_file = argv[1];
-  std::string output_file;
+  std::string input_file_name = argv[1];
+  std::string output_file_name;
 
   if (argc == 3) {
-    output_file = argv[2];
+    output_file_name = argv[2];
 
   } else {
     // If output file is not provided, default is <path to .json>+"output.csv"
+    std::size_t end_of_path = input_file_name.rfind("/");
 
-    std::size_t end_of_path = input_file.rfind("/");
     if (end_of_path == std::string::npos) {
-      end_of_path = input_file.rfind("\\");  // For Windows paths (?)
+      end_of_path = input_file_name.rfind("\\");  // For Windows paths (?)
+
       if (end_of_path == std::string::npos) {
-        std::runtime_error("Could not find path to .json file.");
+        std::cout << "[svzerodsolver] Error: Could not create a default output file." << std::endl;
+        return 1;
       }
     }
 
-    output_file = input_file.substr(0, end_of_path) + "output.csv";
+    output_file_name = input_file_name.substr(0, end_of_path) + "output.csv";
+    std::cout << "[svzerodsolver] Output will be written to '" << output_file_name << "'." << std::endl;;
+  }
+
+  std::ifstream input_file(input_file_name);
+
+  if (!input_file.is_open()) {
+    std::cerr << "[svzerodsolver] Error: The input file '" << input_file_name << "' cannot be opened." << std::endl;
+    return 1;
   }
 
-  std::ifstream ifs(input_file);
-  const auto& config = nlohmann::json::parse(ifs);
+  nlohmann::json config;
+
+  try { 
+    config = nlohmann::json::parse(input_file);
+
+  } catch (const nlohmann::json::parse_error& e) {
+    std::cout << "[svzerodsolver] Error: Parsing the input file '" << input_file_name << "' has failed." << std::endl;
+    std::cout << "[svzerodsolver] Details of the parsing error: " << std::endl;
+    std::cout << e.what() << std::endl;
+    return 1;
+  }
 
   auto solver = Solver(config);
   solver.run();
-  solver.write_result_to_csv(output_file);
+  solver.write_result_to_csv(output_file_name);
 
   return 0;
 }
diff --git a/src/algebra/Integrator.cpp b/src/algebra/Integrator.cpp
@@ -59,8 +59,9 @@ Integrator::Integrator(Model* model, double time_step_size, double rho,
   system.reserve(model);
 }
 
+// Must declare default constructord and dedtructor
+// because of Eigen.
 Integrator::Integrator() {}
-
 Integrator::~Integrator() {}
 
 void Integrator::clean() {
@@ -78,7 +79,7 @@ void Integrator::update_params(double time_step_size) {
   model->update_time(system, 0.0);
 }
 
-State Integrator::step(State& old_state, double time) {
+State Integrator::step(const State& old_state, double time) {
   // Predictor + initiator step
   y_af.setZero();
   ydot_am.setZero();

diff --git a/src/algebra/Integrator.h b/src/algebra/Integrator.h
@@ -106,26 +106,26 @@
  */
 class Integrator {
  private:
-  double alpha_m;
-  double alpha_f;
-  double alpha_m_inv;
-  double alpha_f_inv;
-  double gamma;
-  double gamma_inv;
-  double time_step_size;
-  double time_step_size_inv;
-  double y_dot_coeff;
-  double atol;
-  double y_init_coeff;
-  double ydot_init_coeff;
-  int max_iter;
-  int size;
-  int n_iter = 0;
-  int n_nonlin_iter = 0;
+  double alpha_m{0.0};
+  double alpha_f{0.0};
+  double alpha_m_inv{0.0};
+  double alpha_f_inv{0.0};
+  double gamma{0.0};
+  double gamma_inv{0.0};
+  double time_step_size{0.0};
+  double time_step_size_inv{0.0};
+  double y_dot_coeff{0.0};
+  double atol{0.0};
+  double y_init_coeff{0.0};
+  double ydot_init_coeff{0.0};
+  int max_iter{0};
+  int size{0};
+  int n_iter{0};
+  int n_nonlin_iter{0};
   Eigen::Matrix<double, Eigen::Dynamic, 1> y_af;
   Eigen::Matrix<double, Eigen::Dynamic, 1> ydot_am;
   SparseSystem system;
-  Model *model;
+  Model* model{nullptr};
 
  public:
   /**
@@ -137,7 +137,7 @@ class Integrator {
    * @param atol Absolut tolerance for non-linear iteration termination
    * @param max_iter Maximum number of non-linear iterations
    */
-  Integrator(Model *model, double time_step_size, double rho, double atol,
+  Integrator(Model* model, double time_step_size, double rho, double atol,
              int max_iter);
 
   /**
@@ -173,7 +173,7 @@ class Integrator {
    * @param time Current time
    * @return New state
    */
-  State step(State &state, double time);
+  State step(const State& state, double time);
 
   /**
    * @brief Get average number of nonlinear iterations in all step calls

diff --git a/src/algebra/SparseSystem.cpp b/src/algebra/SparseSystem.cpp
@@ -34,7 +34,7 @@
 
 SparseSystem::SparseSystem() {}
 
-SparseSystem::SparseSystem(unsigned int n) {
+SparseSystem::SparseSystem(int n) {
   F = Eigen::SparseMatrix<double>(n, n);
   E = Eigen::SparseMatrix<double>(n, n);
   dC_dy = Eigen::SparseMatrix<double>(n, n);
@@ -56,10 +56,11 @@ void SparseSystem::clean() {
 
 void SparseSystem::reserve(Model *model) {
   auto num_triplets = model->get_num_triplets();
-  F.reserve(num_triplets["F"]);
-  E.reserve(num_triplets["E"]);
-  dC_dy.reserve(num_triplets["D"]);
-  dC_dydot.reserve(num_triplets["D"]);
+  F.reserve(num_triplets.F);
+  E.reserve(num_triplets.E);
+  dC_dy.reserve(num_triplets.D);
+  dC_dydot.reserve(num_triplets.D);
+
   model->update_constant(*this);
   model->update_time(*this, 0.0);
 
@@ -75,7 +76,7 @@ void SparseSystem::reserve(Model *model) {
   E.makeCompressed();
   dC_dy.makeCompressed();
   dC_dydot.makeCompressed();
-  jacobian.reserve(num_triplets["F"] + num_triplets["E"]);  // Just an estimate
+  jacobian.reserve(num_triplets.F + num_triplets.E);  // Just an estimate
   update_jacobian(1.0);  // Update it once to have sparsity pattern
   jacobian.makeCompressed();
   solver->analyzePattern(jacobian);  // Let solver analyze pattern

diff --git a/src/algebra/SparseSystem.h b/src/algebra/SparseSystem.h
@@ -96,7 +96,7 @@ class SparseSystem {
    *
    * @param n Size of the system
    */
-  SparseSystem(unsigned int n);
+  SparseSystem(int n);
 
   /**
    * @brief Destroy the Sparse System object

diff --git a/src/algebra/State.cpp b/src/algebra/State.cpp
@@ -32,7 +32,7 @@
 
 State::State() {}
 
-State::State(unsigned int n) {
+State::State(int n) {
   y = Eigen::Matrix<double, Eigen::Dynamic, 1>(n);
   ydot = Eigen::Matrix<double, Eigen::Dynamic, 1>(n);
 }
@@ -44,7 +44,8 @@ State::State(const State &state) {
   ydot = state.ydot;
 }
 
-State State::Zero(unsigned int n) {
+State State::Zero(int n) {
+  // [TODO] what's going on here, returing a static State?
   static State state(n);
   state.y = Eigen::Matrix<double, Eigen::Dynamic, 1>::Zero(n);
   state.ydot = Eigen::Matrix<double, Eigen::Dynamic, 1>::Zero(n);

diff --git a/src/algebra/State.h b/src/algebra/State.h
@@ -60,7 +60,7 @@ class State {
    *
    * @param n Size of the state
    */
-  State(unsigned int n);
+  State(int n);
 
   /**
    * @brief Destroy the State object
@@ -81,7 +81,7 @@ class State {
    * @param n Size of the state
    * @return New state initialized with all zeros
    */
-  static State Zero(unsigned int n);
+  static State Zero(int n);
 };
 
 #endif  // SVZERODSOLVER_ALGEBRA_STATE_HPP_