Update travis ci images; fix failfast option in runAmiciSimulations; …

…fixes for cppcheck update (#730)

* Update travis ci images (ubuntu bionic, xcode11)

* Run cppcheck on OSX as the version available for ubuntu bionic reports false positives

* Fix cppcheck errors after cppcheck update

* Test failed after fixing failfast option in previous commit. Fix test with failfast=False.

.travis.yml

@@ -9,7 +9,7 @@ matrix:
   fast_finish: true
   include:
     - os: linux
-      dist: xenial
+      dist: bionic
       language: python
       python: 3.7
       compiler: gcc
@@ -23,7 +23,6 @@ matrix:
             - libboost-serialization-dev
             - swig3.0
             - g++-5
-            - cppcheck
             - libc6-dbg
       env:
         - ENABLE_GCOV_COVERAGE=TRUE
@@ -44,7 +43,7 @@ matrix:
           tags: true
 
     - os: osx
-      osx_image: xcode10.1
+      osx_image: xcode11
       language: minimal
       compiler: clang
       env:
@@ -55,6 +54,7 @@ matrix:
             - hdf5
             - swig
             - gcc
+            - cppcheck
           update: true
       before_install:
         - travis_wait brew link --overwrite gcc # fix linker warning regarding /usr/local/include/c++
@@ -64,7 +64,7 @@ matrix:
         - cd $BASE_DIR # cd to base dir for correct relative path in deploy
 
     - os: osx
-      osx_image: xcode10.1
+      osx_image: xcode11
       language: minimal
       compiler: clang
       env:
@@ -117,7 +117,7 @@ script:
     - if [[ "$TRAVIS_OS_NAME" != "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD python-tests ./scripts/run-python-tests.sh; fi
     - if [[ "$TRAVIS_OS_NAME" != "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD cmake ./tests/testCMakeCompilation.sh; fi
     - if [[ "$CI_MODE" == "deploy" ]]; then $FOLD doxygen ./scripts/run-doxygen.sh; fi
-    - if [[ "$TRAVIS_OS_NAME" == "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD cppcheck ./scripts/run-cppcheck.sh; fi
+    - if [[ "$TRAVIS_OS_NAME" != "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD cppcheck ./scripts/run-cppcheck.sh; fi
     - if [[ "$TRAVIS_OS_NAME" == "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD valgrind ./scripts/run-valgrind.sh; fi
     - if [[ "$TRAVIS_OS_NAME" == "linux" ]] && [[ "$CI_MODE" == "test" ]]; then $FOLD codecov ./scripts/run-codecov.sh; fi
     - if [[ "$TRAVIS_OS_NAME" == "linux" ]] && [[ "$TRAVIS_EVENT_TYPE" == "cron" ]]; then $FOLD sbmltestsuite ./scripts/run-SBMLTestsuite.sh; fi

include/amici/edata.h

@@ -498,7 +498,7 @@ class ConditionContext {
      * @param model
      * @param edata
      */
-    ConditionContext(Model *model, const ExpData *edata = nullptr);
+    explicit ConditionContext(Model *model, const ExpData *edata = nullptr);
 
     ~ConditionContext();
 

include/amici/solver.h

@@ -630,13 +630,13 @@ class Solver {
      * @return t
      */
     realtype gett() const;
-    
+
     /**
      * @brief Reads out the cpu time needed for forward solve
      * @return cpu_time
      */
     realtype getCpuTime() const;
-    
+
     /**
      * @brief Reads out the cpu time needed for bavkward solve
      * @return cpu_timeB
@@ -1424,10 +1424,10 @@ class Solver {
 
     /** relative tolerances for steadystate computation */
     realtype ss_rtol_sensi = NAN;
-    
+
     /** CPU time, forward solve */
     mutable realtype cpu_time = 0.0;
-    
+
     /** CPU time, backward solve */
     mutable realtype cpu_timeB = 0.0;
 
@@ -1455,7 +1455,7 @@ class Solver {
     mutable std::vector<bool> initializedQB{false};
 
     /** number of checkpoints in the forward problem */
-    mutable int ncheckPtr;
+    mutable int ncheckPtr = 0;
 };
 
 bool operator==(const Solver &a, const Solver &b);

include/amici/sundials_linsol_wrapper.h

@@ -380,8 +380,8 @@ class SUNLinSolSPBCGS : public SUNLinSolWrapper {
      * PREC_BOTH)
      * @param maxl Maximum number of solver iterations
      */
-    SUNLinSolSPBCGS(N_Vector x, int pretype = PREC_NONE,
-                    int maxl = SUNSPBCGS_MAXL_DEFAULT);
+    explicit SUNLinSolSPBCGS(N_Vector x, int pretype = PREC_NONE,
+                             int maxl = SUNSPBCGS_MAXL_DEFAULT);
 
     /**
      * @brief SUNLinSolSPBCGS
@@ -390,8 +390,8 @@ class SUNLinSolSPBCGS : public SUNLinSolWrapper {
      * PREC_BOTH)
      * @param maxl Maximum number of solver iterations
      */
-    SUNLinSolSPBCGS(AmiVector const &x, int pretype = PREC_NONE,
-                    int maxl = SUNSPBCGS_MAXL_DEFAULT);
+    explicit SUNLinSolSPBCGS(AmiVector const &x, int pretype = PREC_NONE,
+                             int maxl = SUNSPBCGS_MAXL_DEFAULT);
 
     /**
      * @brief Sets the function pointer for ATimes
@@ -523,8 +523,8 @@ class SUNLinSolSPGMR : public SUNLinSolWrapper {
      * PREC_BOTH)
      * @param maxl Maximum number of solver iterations
      */
-    SUNLinSolSPGMR(AmiVector const &x, int pretype = PREC_NONE,
-                   int maxl = SUNSPGMR_MAXL_DEFAULT);
+    explicit SUNLinSolSPGMR(AmiVector const &x, int pretype = PREC_NONE,
+                            int maxl = SUNSPGMR_MAXL_DEFAULT);
 
     /**
      * @brief Sets the function pointer for ATimes
@@ -590,8 +590,8 @@ class SUNLinSolSPTFQMR : public SUNLinSolWrapper {
      * PREC_BOTH)
      * @param maxl Maximum number of solver iterations
      */
-    SUNLinSolSPTFQMR(N_Vector x, int pretype = PREC_NONE,
-                     int maxl = SUNSPTFQMR_MAXL_DEFAULT);
+    explicit SUNLinSolSPTFQMR(N_Vector x, int pretype = PREC_NONE,
+                              int maxl = SUNSPTFQMR_MAXL_DEFAULT);
 
     /**
      * @brief Create SPTFQMR solver
@@ -600,8 +600,8 @@ class SUNLinSolSPTFQMR : public SUNLinSolWrapper {
      * PREC_BOTH)
      * @param maxl Maximum number of solver iterations
      */
-    SUNLinSolSPTFQMR(AmiVector const &x, int pretype = PREC_NONE,
-                     int maxl = SUNSPTFQMR_MAXL_DEFAULT);
+    explicit SUNLinSolSPTFQMR(AmiVector const &x, int pretype = PREC_NONE,
+                              int maxl = SUNSPTFQMR_MAXL_DEFAULT);
 
     /**
      * @brief Sets the function pointer for ATimes
@@ -837,7 +837,7 @@ class SUNNonLinSolFixedPoint : public SUNNonLinSolWrapper {
      * @param x template for cloning vectors needed within the solver.
      * @param m number of acceleration vectors to use
      */
-    SUNNonLinSolFixedPoint(const_N_Vector x, int m = 0);
+    explicit SUNNonLinSolFixedPoint(const_N_Vector x, int m = 0);
 
     /**
      * @brief Create fixed-point solver for use with sensitivity analysis

include/amici/sundials_matrix_wrapper.h

@@ -76,7 +76,7 @@ class SUNMatrixWrapper {
      * @brief Move constructor
      * @param other
      */
-    SUNMatrixWrapper(SUNMatrixWrapper &&other) noexcept;
+    SUNMatrixWrapper(SUNMatrixWrapper &&other);
 
     /**
      * @brief Copy assignment
@@ -90,7 +90,7 @@ class SUNMatrixWrapper {
      * @param other
      * @return
      */
-    SUNMatrixWrapper &operator=(SUNMatrixWrapper &&other) noexcept;
+    SUNMatrixWrapper &operator=(SUNMatrixWrapper &&other);
 
     /**
      * @brief Access raw data

include/amici/vector.h

@@ -38,10 +38,10 @@ class AmiVector {
         : vec(static_cast<decltype(vec)::size_type>(length), 0.0),
           nvec(N_VMake_Serial(length, vec.data())) {}
 
-    /** Copies data from std::vector and constructs an nvec that points to the
+    /** Moves data from std::vector and constructs an nvec that points to the
      * data
      * @brief constructor from std::vector,
-     * @param rvec vector from which the data will be copied
+     * @param rvec vector from which the data will be moved
      * @return new AmiVector instance
      */
     explicit AmiVector(std::vector<realtype> rvec)

src/amici.cpp

@@ -122,7 +122,9 @@ std::vector<std::unique_ptr<ReturnData> > runAmiciSimulations(const Solver &solv
 )
 {
     std::vector<std::unique_ptr<ReturnData> > results(edatas.size());
-    bool failed = false;
+    // is set to true if one simulation fails and we should skip the rest.
+    // shared across threads.
+    bool skipThrough = false;
 
 #if defined(_OPENMP)
     #pragma omp parallel for num_threads(num_threads)
@@ -133,16 +135,15 @@ std::vector<std::unique_ptr<ReturnData> > runAmiciSimulations(const Solver &solv
 
         /* if we fail we need to write empty return datas for the python
          interface */
-        if (failed) {
+        if (skipThrough) {
             ConditionContext conditionContext(myModel.get(), edatas[i]);
             results[i] =
                 std::unique_ptr<ReturnData>(new ReturnData(solver, model));
+        } else {
+            results[i] = runAmiciSimulation(*mySolver, edatas[i], *myModel);
         }
 
-        results[i] = runAmiciSimulation(*mySolver, edatas[i], *myModel);
-
-        if (results[i]->status < 0 && failfast)
-            failed = true;
+        skipThrough |= failfast && results[i]->status < 0;
     }
 
     return results;

src/newton_solver.cpp

@@ -254,7 +254,6 @@ void NewtonSolverIterative::linsolveSPBCG(int ntry, int nnewt,
     // ns_r = xdot - ns_Jv;
     N_VLinearSum(-1.0, ns_Jv.getNVector(), 1.0, xdot.getNVector(), ns_r.getNVector());
     N_VDiv(ns_r.getNVector(), ns_Jdiag.getNVector(), ns_r.getNVector());
-    double res = sqrt(N_VDotProd(ns_r.getNVector(), ns_r.getNVector()));
     ns_rt = ns_r;
 
     for (int i_linstep = 0; i_linstep < maxlinsteps;
@@ -296,7 +295,7 @@ void NewtonSolverIterative::linsolveSPBCG(int ntry, int nnewt,
 
         // Compute the (unscaled) residual
         N_VProd(ns_r.getNVector(), ns_Jdiag.getNVector(), ns_r.getNVector());
-        res = sqrt(N_VDotProd(ns_r.getNVector(), ns_r.getNVector()));
+        double res = sqrt(N_VDotProd(ns_r.getNVector(), ns_r.getNVector()));
 
         // Test convergence
         if (res < atol) {

src/solver.cpp

@@ -14,36 +14,20 @@ namespace amici {
 
 extern msgIdAndTxtFp warnMsgIdAndTxt;
 
-Solver::Solver(const Solver &other) : Solver() {
-    t = nan("");
-    ncheckPtr = 0;
-    sensi = other.sensi;
-    atol = other.atol;
-    rtol = other.rtol;
-    atol_fsa = other.atol_fsa;
-    rtol_fsa = other.rtol_fsa;
-    atolB = other.atolB;
-    rtolB = other.rtolB;
-    quad_atol = other.quad_atol;
-    quad_rtol = other.quad_rtol;
-    ss_atol = other.ss_atol;
-    ss_rtol = other.ss_rtol;
-    ss_atol_sensi = other.ss_atol_sensi;
-    ss_rtol_sensi = other.ss_rtol_sensi;
-    maxsteps = other.maxsteps;
-    maxstepsB = other.maxstepsB;
-    newton_maxsteps = other.newton_maxsteps;
-    newton_maxlinsteps = other.newton_maxlinsteps;
-    newton_preeq = other.newton_preeq;
-    ism = other.ism;
-    sensi_meth = other.sensi_meth;
-    linsol = other.linsol;
-    interpType = other.interpType;
-    lmm = other.lmm;
-    iter = other.iter;
-    stldet = other.stldet;
-    ordering = other.ordering;
-}
+Solver::Solver(const Solver &other)
+    : ism(other.ism), lmm(other.lmm), iter(other.iter),
+      interpType(other.interpType), maxsteps(other.maxsteps), t(nan("")),
+      sensi_meth(other.sensi_meth), stldet(other.stldet),
+      ordering(other.ordering), newton_maxsteps(other.newton_maxsteps),
+      newton_maxlinsteps(other.newton_maxlinsteps),
+      newton_preeq(other.newton_preeq), linsol(other.linsol),
+      atol(other.atol), rtol(other.rtol), atol_fsa(other.atol_fsa),
+      rtol_fsa(other.rtol_fsa), atolB(other.atolB), rtolB(other.rtolB),
+      quad_atol(other.quad_atol), quad_rtol(other.quad_rtol),
+      ss_atol(other.ss_atol), ss_rtol(other.ss_rtol),
+      ss_atol_sensi(other.ss_atol_sensi), ss_rtol_sensi(other.ss_rtol_sensi),
+      maxstepsB(other.maxstepsB), sensi(other.sensi)
+{}
 
 int Solver::run(const realtype tout) const {
     setStopTime(tout);

src/sundials_matrix_wrapper.cpp

@@ -82,7 +82,7 @@ SUNMatrixWrapper::SUNMatrixWrapper(const SUNMatrixWrapper &other) {
     update_ptrs();
 }
 
-SUNMatrixWrapper::SUNMatrixWrapper(SUNMatrixWrapper &&other) noexcept {
+SUNMatrixWrapper::SUNMatrixWrapper(SUNMatrixWrapper &&other) {
     std::swap(matrix, other.matrix);
     update_ptrs();
 }
@@ -94,7 +94,7 @@ SUNMatrixWrapper &SUNMatrixWrapper::operator=(const SUNMatrixWrapper &other) {
 }
 
 SUNMatrixWrapper &SUNMatrixWrapper::
-operator=(SUNMatrixWrapper &&other) noexcept {
+operator=(SUNMatrixWrapper &&other) {
     std::swap(matrix, other.matrix);
     update_ptrs();
     return *this;

src/vector.cpp

@@ -23,8 +23,8 @@ int AmiVector::getLength() const { return static_cast<int>(vec.size()); }
 void AmiVector::reset() { set(0.0); }
 
 void AmiVector::minus() {
-    for (auto & it : vec)
-        it = -it;
+    std::transform(vec.begin(), vec.end(),
+                   vec.begin(), std::negate<realtype>());
 }
 
 void AmiVector::set(realtype val) { std::fill(vec.begin(), vec.end(), val); }

tests/testModels.py

@@ -126,7 +126,8 @@ def assert_fun(x):
                             edatas = [edata.get(), edata.get()]
 
                         rdatas = amici.runAmiciSimulations(
-                            self.model, self.solver, edatas, num_threads=2
+                            self.model, self.solver, edatas, num_threads=2,
+                            failfast=False
                         )
                         verify_simulation_results(
                             rdatas[0],