Fix Visual C++ issues, update cppcheck handling, cleanup

* use c++ standard attributes, fixes #803

* avoid use of '..' in paths, fixes 805

* fix cppcheck issues (add consts, ignore alloca obsolete)

* Fix(core) Include header for std::negate

* Fix(ci) cppcheck should ignore alloca usage

* Cleanup python/amici/__init__.py and fix doxygen issue

* Can we simplify scripts/run-cppcheck.sh?

* Remove obsolete function ‘int amici::fJDiag(amici::realtype, N_Vector, N_Vector, void*)’

* Formatting

.cppcheck-exitcode-suppressions

@@ -0,0 +1 @@
+alloca:*:*

include/amici/model.h

@@ -183,15 +183,15 @@ class Model : public AbstractModel {
      * @param sx pointer to state variable sensititivies
      * @param x pointer to state variables
      */
-    void initializeStateSensitivities(AmiVectorArray &sx, AmiVector &x);
+    void initializeStateSensitivities(AmiVectorArray &sx, const AmiVector &x);
 
     /**
      * Initialises the heaviside variables h at the intial time t0
      * heaviside variables activate/deactivate on event occurences
      * @param x pointer to state variables
      * @param dx pointer to time derivative of states (DAE only)
      */
-    void initHeaviside(AmiVector &x, AmiVector &dx);
+    void initHeaviside(const AmiVector &x, const AmiVector &dx);
 
     /**
      * @brief Number of parameters wrt to which sensitivities are computed

python/amici/__init__.py

@@ -32,6 +32,38 @@
 import sys
 from contextlib import suppress
 
+
+def _get_amici_path():
+    """
+    Determine package installation path, or, if used directly from git
+    repository, get repository root
+    """
+    basedir = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+    if os.path.exists(os.path.join(basedir, '.git')):
+        return os.path.abspath(basedir)
+    return os.path.dirname(__file__)
+
+
+def _get_commit_hash():
+    """Get commit hash from file"""
+    basedir = os.path.dirname(os.path.dirname(os.path.dirname(amici_path)))
+    commitfile = next(
+        (
+            file for file in [
+                os.path.join(basedir, '.git', 'FETCH_HEAD'),
+                os.path.join(basedir, '.git', 'ORIG_HEAD'),
+            ]
+            if os.path.isfile(file)
+        ),
+        None
+    )
+
+    if commitfile:
+        with open(commitfile) as f:
+            return str(re.search(r'^([\w]*)', f.read().strip()).group())
+    return 'unknown'
+
+
 # redirect C/C++ stdout to python stdout if python stdout is redirected,
 # e.g. in ipython notebook
 capture_cstdout = suppress
@@ -57,14 +89,8 @@
     except (ImportError, ModuleNotFoundError, AttributeError):
         pass
 
-# determine package installation path, or, if used directly from git
-# repository, get repository root
-if os.path.exists(os.path.join(os.path.dirname(__file__), '..', '..', '.git')):
-    amici_path = os.path.abspath(os.path.join(
-        os.path.dirname(__file__), '..', '..'))
-else:
-    amici_path = os.path.dirname(__file__)
-
+# Initialize AMICI paths
+amici_path = _get_amici_path()
 amiciSwigPath = os.path.join(amici_path, 'swig')
 amiciSrcPath = os.path.join(amici_path, 'src')
 amiciModulePath = os.path.dirname(__file__)
@@ -73,24 +99,7 @@
 with open(os.path.join(amici_path, 'version.txt')) as f:
     __version__ = f.read().strip()
 
-# get commit hash from file
-_commitfile = next(
-    (
-        file for file in [
-            os.path.join(amici_path, '..', '..', '..', '.git', 'FETCH_HEAD'),
-            os.path.join(amici_path, '..', '..', '..', '.git', 'ORIG_HEAD'),
-        ]
-        if os.path.isfile(file)
-    ),
-    None
-)
-
-if _commitfile:
-    with open(_commitfile) as f:
-        __commit__ = str(re.search(r'^([\w]*)', f.read().strip()).group())
-else:
-    __commit__ = 'unknown'
-
+__commit__ = _get_commit_hash()
 
 try:
     # These module require the swig interface and other dependencies which will

scripts/run-cppcheck.sh

@@ -3,46 +3,13 @@
 # Note: CppuTest memcheck should be disabled
 # Note: Consider using ctest -T memcheck instead
 
-SCRIPT_PATH=$(dirname $BASH_SOURCE)
-AMICI_PATH=$(cd $SCRIPT_PATH/.. && pwd)
+SCRIPT_PATH=$(dirname "$BASH_SOURCE")
+AMICI_PATH=$(cd "$SCRIPT_PATH"/.. && pwd)
 
 cd ${AMICI_PATH}
 
-cppcheck -i${AMICI_PATH}/src/doc ${AMICI_PATH}/src  -I${AMICI_PATH}/include/ --enable=style 2> cppcheck.txt
+cppcheck -i"${AMICI_PATH}"/src/doc "${AMICI_PATH}"/src \
+         -I$"{AMICI_PATH}"/include/ \
+         --enable=style \
+         --exitcode-suppressions="${AMICI_PATH}"/.cppcheck-exitcode-suppressions
 
-# suppress alloca warnings
-grep -v "(warning) Obsolete function 'alloca' called." cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-
-# suppress header warnings for standard libraries
-grep -v "Cppcheck cannot find all the include files" cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-grep -v "'AmiVectorArray' does not have a operator=" cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-grep -v "Member variable 'ExpData::nytrue_' is not initialized in the constructor" cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-grep -v "Member variable 'ExpData::nztrue_' is not initialized in the constructor" cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-grep -v "Member variable 'ExpData::nmaxevent_' is not initialized in the constructor" cppcheck.txt > cppcheck_tmp.txt
-mv cppcheck_tmp.txt cppcheck.txt
-
-# check if error log was created
-if [ -f cppcheck.txt  ]; then
-    # check if error log is empty
-    if [ -s cppcheck.txt ]; then
-        echo "CPPCHECK failed:"
-        cat cppcheck.txt
-        rm cppcheck.txt
-        exit 1
-    else
-        rm cppcheck.txt
-        exit 0
-    fi
-else
-    exit 1
-fi

src/model.cpp

@@ -216,7 +216,8 @@ void Model::initializeStates(AmiVector &x) {
     }
 }
 
-void Model::initializeStateSensitivities(AmiVectorArray &sx, AmiVector &x) {
+void Model::initializeStateSensitivities(AmiVectorArray &sx,
+                                         const AmiVector &x) {
     if (sx0data.empty()) {
         fsx0(sx, x);
     } else {
@@ -234,7 +235,7 @@ void Model::initializeStateSensitivities(AmiVectorArray &sx, AmiVector &x) {
     }
 }
 
-void Model::initHeaviside(AmiVector &x, AmiVector &dx) {
+void Model::initHeaviside(const AmiVector &x, const AmiVector &dx) {
     std::vector<realtype> rootvals(ne, 0.0);
     froot(tstart, x, dx, rootvals);
     for (int ie = 0; ie < ne; ie++) {

src/solver_cvodes.cpp

@@ -64,9 +64,6 @@ static int fJBandB(realtype t, N_Vector x, N_Vector xB, N_Vector xBdot,
                    SUNMatrix JB, void *user_data, N_Vector tmp1B,
                    N_Vector tmp2B, N_Vector tmp3B);
 
-static int fJDiag(realtype t, N_Vector JDiag, N_Vector x, void *user_data)
-__attribute__((unused));
-
 static int fJv(N_Vector v, N_Vector Jv, realtype t, N_Vector x,
                N_Vector xdot, void *user_data, N_Vector tmp);
 
@@ -839,19 +836,6 @@ int fJBandB(realtype t, N_Vector x, N_Vector xB, N_Vector xBdot,
     return fJB(t, x, xB, xBdot, JB, user_data, tmp1B, tmp2B, tmp3B);
 }
 
-/**
- * @brief Diagonalized Jacobian (for preconditioning)
- * @param t timepoint
- * @param JDiag Vector to which the Jacobian diagonal will be written
- * @param x Vector with the states
- * @param user_data object with user input @type Model_ODE
- **/
-int fJDiag(realtype t, N_Vector JDiag, N_Vector x, void *user_data) {
-    auto model = static_cast<Model_ODE *>(user_data);
-    model->fJDiag(t, JDiag, x);
-    return model->checkFinite(gsl::make_span(JDiag), "Jacobian");
-}
-
 /**
  * @brief Matrix vector product of J with a vector v (for iterative solvers)
  * @param t timepoint

src/vector.cpp

@@ -1,5 +1,7 @@
 #include "amici/vector.h"
 
+#include <functional>
+
 namespace amici {
 
 AmiVector &AmiVector::operator=(AmiVector const &other) {