Make compile -Wall/-Werror proof (#653)

* Try resolving issues discovered with -Wall -Werror * Remove unused private data members, resolve overridden virtual functions * try to fix the CI tests on github (#654) * try to fix the CI tests on github * try to fix the CI tests on github * add deepcheck * pass quartz CI * build on lassen * remove some unused var * fix PNNL CI v1 * try to fix ginkgo issue * create branch, switch to cpp14 (#661) * rebase and fix issues introduced by rebase --------- Co-authored-by: Nai-Yuan Chiang <[email protected]> Co-authored-by: nychiang <[email protected]>
LLNL · Oct 2, 2023 · 2919a87 · 2919a87
1 parent 8877fa8
commit 2919a87
Showing 77 changed files with 442 additions and 541 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -6,6 +6,7 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
 set(CMAKE_CUDA_SEPARABLE_COMPILATION ON)
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+set(CMAKE_CXX_FLAGS "-Wall -Werror")
 
 if (POLICY CMP0074)
   cmake_policy(SET CMP0074 NEW)

diff --git a/src/Drivers/Dense/NlpDenseConsEx1.cpp b/src/Drivers/Dense/NlpDenseConsEx1.cpp
@@ -106,23 +106,29 @@ DiscretizedFunction::DiscretizedFunction(Ex1Meshing1D* meshing)
 }
 
 // u'*v = u'*M*v, where u is 'this'
-double DiscretizedFunction::dotProductWith( const DiscretizedFunction& v_ ) const
+double DiscretizedFunction::dotProductWith( const hiopVector& v_ ) const
 {
-  assert(v_._mesh->matches(this->_mesh));
-  double* M=_mesh->_mass->local_data();
-  double* u= this->data_;
-  double* v= v_.data_;
+  auto discretizedFunction(dynamic_cast<const DiscretizedFunction*>(&v_));
+  if (discretizedFunction) {
+    assert(discretizedFunction->_mesh->matches(this->_mesh));
+    double* M=_mesh->_mass->local_data();
+    double* u= this->data_;
+    double* v= discretizedFunction->data_;
 
-  double dot=0.;
-  for(int i=0; i<get_local_size(); i++)
-    dot += u[i]*M[i]*v[i];
+    double dot=0.;
+    for(int i=0; i<get_local_size(); i++)
+      dot += u[i]*M[i]*v[i];
 
  #ifdef HIOP_USE_MPI
-  double dotprodG;
-  int ierr = MPI_Allreduce(&dot, &dotprodG, 1, MPI_DOUBLE, MPI_SUM, comm_); assert(MPI_SUCCESS==ierr);
-  dot=dotprodG;
+    double dotprodG;
+    int ierr = MPI_Allreduce(&dot, &dotprodG, 1, MPI_DOUBLE, MPI_SUM, comm_); assert(MPI_SUCCESS==ierr);
+    dot=dotprodG;
 #endif
-  return dot;
+    return dot;
+  }
+  else {
+    return hiopVectorPar::dotProductWith(v_);
+  }
 }
 
 // computes integral of 'this', that is sum (this[elem]*m[elem])

diff --git a/src/Drivers/Dense/NlpDenseConsEx1.hpp b/src/Drivers/Dense/NlpDenseConsEx1.hpp
@@ -91,9 +91,9 @@ class DiscretizedFunction : public hiop::hiopVectorPar
 public:
   DiscretizedFunction(Ex1Meshing1D* meshing);
 
-  virtual double dotProductWith( const DiscretizedFunction& v ) const;
+  double dotProductWith( const hiopVector& v ) const override;
   virtual double integral() const;
-  virtual double twonorm() const;
+  double twonorm() const override;
 
   /* the following methods are mostly for educational purposes and may not be optimized */
   //converts the local indexes to global indexes
@@ -112,7 +112,7 @@ class DenseConsEx1 : public hiop::hiopInterfaceDenseConstraints
 {
 public: 
   DenseConsEx1(int n_mesh_elem=100, double mesh_ratio=1.0)
-    : n_vars(n_mesh_elem), n_cons(0), comm(MPI_COMM_WORLD)
+    : n_vars(n_mesh_elem), comm(MPI_COMM_WORLD)
   {
     //create the members
     _mesh = new Ex1Meshing1D(0.0,1.0, n_vars, mesh_ratio, comm);
@@ -219,7 +219,7 @@ class DenseConsEx1 : public hiop::hiopInterfaceDenseConstraints
     return true;
   }
 private:
-  int n_vars, n_cons;
+  int n_vars;
   MPI_Comm comm;
   Ex1Meshing1D* _mesh;
 

diff --git a/src/Drivers/Dense/NlpDenseConsEx1Driver.cpp b/src/Drivers/Dense/NlpDenseConsEx1Driver.cpp
@@ -60,8 +60,9 @@ static void usage(const char* exeName)
 
 int main(int argc, char **argv)
 {
-  int rank=0, numRanks=1;
+  int rank = 0;
 #ifdef HIOP_USE_MPI
+  int numRanks = 1;
   int err;
   err = MPI_Init(&argc, &argv);                  assert(MPI_SUCCESS==err);
   err = MPI_Comm_rank(MPI_COMM_WORLD,&rank);     assert(MPI_SUCCESS==err);

diff --git a/src/Drivers/Dense/NlpDenseConsEx2.cpp b/src/Drivers/Dense/NlpDenseConsEx2.cpp
@@ -5,14 +5,14 @@
 #include <cstdio>
 
 DenseConsEx2::DenseConsEx2(int n, bool unconstrained)
-  : unconstrained_(unconstrained),
-    n_vars_(n),
+  : n_vars_(n),
     n_cons_(4),
-    comm(MPI_COMM_WORLD)
+    unconstrained_(unconstrained)
 {
   comm_size = 1;
   my_rank = 0; 
 #ifdef HIOP_USE_MPI
+  comm = MPI_COMM_WORLD;
   int ierr = MPI_Comm_size(comm, &comm_size); assert(MPI_SUCCESS==ierr);
   ierr = MPI_Comm_rank(comm, &my_rank); assert(MPI_SUCCESS==ierr);
 #endif

diff --git a/src/Drivers/Dense/NlpDenseConsEx2.hpp b/src/Drivers/Dense/NlpDenseConsEx2.hpp
@@ -90,7 +90,9 @@ class DenseConsEx2 : public hiop::hiopInterfaceDenseConstraints
 */
 private:
   size_type n_vars_, n_cons_;
+#ifdef HIOP_USE_MPI
   MPI_Comm comm;
+#endif
   int my_rank;
   int comm_size;
   index_type* col_partition_;

diff --git a/src/Drivers/Dense/NlpDenseConsEx3.hpp b/src/Drivers/Dense/NlpDenseConsEx3.hpp
@@ -33,10 +33,11 @@ class DenseConsEx3 : public hiop::hiopInterfaceDenseConstraints
 {
 public: 
   DenseConsEx3(int n)
-    : n_vars(n), n_cons(2), comm(MPI_COMM_WORLD)
+    : n_vars(n), n_cons(2)
   {
     comm_size=1; my_rank=0; 
 #ifdef HIOP_USE_MPI
+    comm = MPI_COMM_WORLD;
     int ierr = MPI_Comm_size(comm, &comm_size); assert(MPI_SUCCESS==ierr);
     ierr = MPI_Comm_rank(comm, &my_rank); assert(MPI_SUCCESS==ierr);
 #endif
@@ -226,7 +227,9 @@ class DenseConsEx3 : public hiop::hiopInterfaceDenseConstraints
 
 private:
   int n_vars, n_cons;
+#ifdef HIOP_USE_MPI
   MPI_Comm comm;
+#endif
   int my_rank, comm_size;
   index_type* col_partition;
 public:

diff --git a/src/Drivers/Dense/NlpDenseConsEx4.cpp b/src/Drivers/Dense/NlpDenseConsEx4.cpp
@@ -5,14 +5,14 @@
 #include <cstdio>
 
 DenseConsEx4::DenseConsEx4()
-  : unconstrained_(false),
-    n_vars_(2),
+  : n_vars_(2),
     n_cons_(4),
-    comm(MPI_COMM_WORLD)
+    unconstrained_(false)
 {
   comm_size = 1;
   my_rank = 0; 
 #ifdef HIOP_USE_MPI
+  comm = MPI_COMM_WORLD;
   int ierr = MPI_Comm_size(comm, &comm_size); assert(MPI_SUCCESS==ierr);
   ierr = MPI_Comm_rank(comm, &my_rank); assert(MPI_SUCCESS==ierr);
 #endif
@@ -96,7 +96,6 @@ bool DenseConsEx4::get_cons_info(const size_type& m, double* clow, double* cupp,
 
 bool DenseConsEx4::eval_f(const size_type& n, const double* x, bool new_x, double& obj_value)
 {
-  size_type n_local = col_partition_[my_rank+1] - col_partition_[my_rank];
   obj_value = 0.;
 
   index_type i_local;
@@ -247,7 +246,6 @@ bool DenseConsEx4::eval_Jac_cons(const size_type& n,
 
   assert(n==n_vars_); assert(m==n_cons_); 
   size_type n_local = col_partition_[my_rank+1] - col_partition_[my_rank];
-  int i;
   //here we will iterate over the local indexes, however we still need to work with the
   //global indexes to correctly determine the entries in the Jacobian corresponding
   //to the 'rebels' variables x_1, x_2, x_3

diff --git a/src/Drivers/Dense/NlpDenseConsEx4.hpp b/src/Drivers/Dense/NlpDenseConsEx4.hpp
@@ -87,7 +87,9 @@ class DenseConsEx4 : public hiop::hiopInterfaceDenseConstraints
 */
 private:
   size_type n_vars_, n_cons_;
+#ifdef HIOP_USE_MPI
   MPI_Comm comm;
+#endif
   int my_rank;
   int comm_size;
   index_type* col_partition_;

diff --git a/src/Drivers/MDS/NlpMdsEx1Driver.cpp b/src/Drivers/MDS/NlpMdsEx1Driver.cpp
@@ -11,7 +11,7 @@
 
 using namespace hiop;
 
-static bool self_check(size_type n, double obj_value);
+// static bool self_check(size_type n, double obj_value);
 
 static bool parse_arguments(int argc, char **argv,
                             bool& self_check,

diff --git a/src/Drivers/MDS/NlpMdsEx2Driver.cpp b/src/Drivers/MDS/NlpMdsEx2Driver.cpp
@@ -11,7 +11,7 @@
 
 using namespace hiop;
 
-static bool self_check(size_type n, double obj_value);
+// static bool self_check(size_type n, double obj_value);
 
 static bool parse_arguments(int argc, char **argv,
                             bool& self_check,

diff --git a/src/Drivers/MDS/NlpMdsRajaEx1.cpp b/src/Drivers/MDS/NlpMdsRajaEx1.cpp
@@ -94,8 +94,8 @@ using hiopMatrixRajaDense = hiop::hiopMatrixDenseRaja<hiop::MemBackendUmpire, hi
 using namespace hiop;
 
 MdsEx1::MdsEx1(int ns_in, int nd_in, std::string mem_space, bool empty_sp_row)
-  : mem_space_(mem_space), 
-    ns_(ns_in),
+  : ns_(ns_in),
+    mem_space_(mem_space), 
     sol_x_(NULL),
     sol_zl_(NULL),
     sol_zu_(NULL),

diff --git a/src/Drivers/MDS/hpc_multisolves.cpp b/src/Drivers/MDS/hpc_multisolves.cpp
@@ -49,7 +49,7 @@ int main(int argc, char *argv[])
     fflush(stdout);
 
     double obj_value=-1e+20;
-    hiopSolveStatus status;
+    [[maybe_unused]] hiopSolveStatus status;
 
     //user's NLP -> implementation of hiop::hiopInterfaceMDS
     MdsEx1* my_nlp = new MdsEx1(n_sp, n_de);

diff --git a/src/Drivers/PriDec/NlpPriDecEx1.cpp b/src/Drivers/PriDec/NlpPriDecEx1.cpp
@@ -211,7 +211,7 @@ solve_master(hiopVector& x,
     }
   }
 
-  bool ierr = my_nlp->set_include(include_r);
+  [[maybe_unused]] bool ierr = my_nlp->set_include(include_r);
   if(include_r) {
     assert(my_nlp->quad_is_defined());
   }
@@ -257,11 +257,11 @@ solve_master(hiopVector& x,
 
 };
 
-bool PriDecMasterProblemEx1::eval_f_rterm(size_t idx, const int& n,const  double* x, double& rval)
+bool PriDecMasterProblemEx1::eval_f_rterm(size_type idx, const int& n,const  double* x, double& rval)
 {
   rval = 0.;
   for(int i=0; i<n; i++) {
-    if(i==idx) {   
+    if(i==static_cast<int>(idx)) {   
       rval += (x[i]+S_)*(x[i]+S_);
     } else {
       rval += x[i]*x[i];
@@ -273,12 +273,12 @@ bool PriDecMasterProblemEx1::eval_f_rterm(size_t idx, const int& n,const  double
 };
 
 // x is handled by primalDecomp to be the correct coupled x
-bool PriDecMasterProblemEx1::eval_grad_rterm(size_t idx, const int& n, double* x, hiopVector& grad)
+bool PriDecMasterProblemEx1::eval_grad_rterm(size_type idx, const int& n, double* x, hiopVector& grad)
 {
-  assert(nc_ == n);
+  assert(static_cast<int>(nc_) == n);
   double* grad_vec = grad.local_data();
   for(int i=0; i<n; i++) {
-    if(i==idx) {   
+    if(i==static_cast<int>(idx)) {   
       grad_vec[i] = (x[i]+S_);
     } else {
       grad_vec[i] = x[i];

diff --git a/src/Drivers/PriDec/NlpPriDecEx1.hpp b/src/Drivers/PriDec/NlpPriDecEx1.hpp
@@ -151,12 +151,12 @@ class PriDecMasterProblemEx1 : public hiopInterfacePriDecProblem
   /**
    * This function returns the recourse objective, which is 0.5*(x+Se_i)(x+Se_i).
    */
-  virtual bool eval_f_rterm(size_t idx, const int& n,const  double* x, double& rval);
+  virtual bool eval_f_rterm(size_type idx, const int& n,const  double* x, double& rval);
 
   /**
    * This function returns the recourse gradient.
    */
-  virtual bool eval_grad_rterm(size_t idx, const int& n, double* x, hiopVector& grad);
+  virtual bool eval_grad_rterm(size_type idx, const int& n, double* x, hiopVector& grad);
 
   /**
    * This function sets up the approximation of the recourse objective based on the function value and gradient 
@@ -167,19 +167,19 @@ class PriDecMasterProblemEx1 : public hiopInterfacePriDecProblem
   virtual bool set_recourse_approx_evaluator(const int n, 
                                              hiopInterfacePriDecProblem::RecourseApproxEvaluator* evaluator);
   // Returns the number S of recourse terms.
-  size_t get_num_rterms() const {return S_;}
-  size_t get_num_vars() const {return n_;}
+  size_type get_num_rterms() const {return S_;}
+  size_type get_num_vars() const {return n_;}
   // Returns the solution.
   void get_solution(double* x) const 
   {
-    for(int i=0; i<n_; i++)
+    for(int i=0; i<static_cast<int>(n_); i++)
       x[i] = sol_[i];
   }
   double get_objective() {return obj_;}
 private:
-  size_t n_;
-  size_t S_;
-  size_t nc_;
+  size_type n_;
+  size_type S_;
+  size_type nc_;
   PriDecEx1* my_nlp;
   double obj_;
   double* sol_; 

diff --git a/src/Drivers/PriDec/NlpPriDecEx2.cpp b/src/Drivers/PriDec/NlpPriDecEx2.cpp
@@ -10,7 +10,7 @@ using namespace hiop;
 
 
 PriDecMasterProbleEx2::
-PriDecMasterProbleEx2(size_t nx, size_t ny, size_t nS, size_t S) : nx_(nx), ny_(ny),nS_(nS),S_(S)
+PriDecMasterProbleEx2(size_type nx, size_type ny, size_type nS, size_type S) : nx_(nx), ny_(ny),nS_(nS),S_(S)
 {
   assert(nx==ny);
   y_ = new double[ny_];
@@ -103,16 +103,17 @@ set_recourse_approx_evaluator(const int n,
   return true; 
 }
 
-bool PriDecMasterProbleEx2::eval_f_rterm(size_t idx, const int& n, const double* x, double& rval)
+bool PriDecMasterProbleEx2::eval_f_rterm(size_type idx, const int& n, const double* x, double& rval)
 {
   assert(nx_==n);
   rval=-1e+20;
-  hiopSolveStatus status;
   double* xi;
+  hiopSolveStatus status;
 
 #ifdef HIOP_USE_MPI
-  double t3 =  MPI_Wtime(); 
-  double t4 = 0.; 
+  // uncomment if want to monitor contingency computing time
+  //double t3 =  MPI_Wtime(); 
+  //double t4 = 0.; 
 #endif 
 
   // xi can be set below 
@@ -149,6 +150,7 @@ bool PriDecMasterProbleEx2::eval_f_rterm(size_t idx, const int& n, const double*
 
   //assert("for debugging" && false); //for debugging purpose
   status = solver.run();
+  assert(status<=hiopSolveStatus::User_Stopped); //check solver status if necessary
   rval = solver.getObjective();  
   if(y_==NULL) {
     y_ = new double[ny_];
@@ -171,7 +173,7 @@ bool PriDecMasterProbleEx2::eval_f_rterm(size_t idx, const int& n, const double*
 };
 
 // returns the gradient computed in eval_f_rterm
-bool PriDecMasterProbleEx2::eval_grad_rterm(size_t idx, const int& n, double* x, hiopVector& grad)
+bool PriDecMasterProbleEx2::eval_grad_rterm(size_type idx, const int& n, double* x, hiopVector& grad)
 {
   assert(nx_==n);
   double* grad_vec = grad.local_data();
@@ -181,12 +183,12 @@ bool PriDecMasterProbleEx2::eval_grad_rterm(size_t idx, const int& n, double* x,
   return true;
 };
 
-inline size_t PriDecMasterProbleEx2::get_num_rterms() const
+inline size_type PriDecMasterProbleEx2::get_num_rterms() const
 {
   return S_;
 }
 
-inline size_t PriDecMasterProbleEx2::get_num_vars() const
+inline size_type PriDecMasterProbleEx2::get_num_vars() const
 {
   return nx_;
 }