From 3807f437e2b41e4fd8df6ec0e210ce9f959e76b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Fr=C3=B6hlich?= Date: Fri, 27 Sep 2019 13:59:22 -0400 Subject: [PATCH] Fix Visual C++ issues, update cppcheck handling, cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 | 1 + include/amici/model.h | 4 +-- python/amici/__init__.py | 61 +++++++++++++++++++-------------- scripts/run-cppcheck.sh | 45 ++++-------------------- src/model.cpp | 5 +-- src/solver_cvodes.cpp | 16 --------- src/vector.cpp | 2 ++ 7 files changed, 49 insertions(+), 85 deletions(-) create mode 100644 .cppcheck-exitcode-suppressions diff --git a/.cppcheck-exitcode-suppressions b/.cppcheck-exitcode-suppressions new file mode 100644 index 0000000000..fcc48d95ea --- /dev/null +++ b/.cppcheck-exitcode-suppressions @@ -0,0 +1 @@ +alloca:*:* diff --git a/include/amici/model.h b/include/amici/model.h index 2f77b152e0..1ab5e5f847 100644 --- a/include/amici/model.h +++ b/include/amici/model.h @@ -183,7 +183,7 @@ 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 @@ -191,7 +191,7 @@ class Model : public AbstractModel { * @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 diff --git a/python/amici/__init__.py b/python/amici/__init__.py index 0fdf6654ad..c26c9ff281 100644 --- a/python/amici/__init__.py +++ b/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 diff --git a/scripts/run-cppcheck.sh b/scripts/run-cppcheck.sh index 721d348620..82018977ea 100755 --- a/scripts/run-cppcheck.sh +++ b/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 diff --git a/src/model.cpp b/src/model.cpp index 9e21e5a6d6..54ba5cb839 100644 --- a/src/model.cpp +++ b/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 rootvals(ne, 0.0); froot(tstart, x, dx, rootvals); for (int ie = 0; ie < ne; ie++) { diff --git a/src/solver_cvodes.cpp b/src/solver_cvodes.cpp index 4267cee5d3..56441f7828 100644 --- a/src/solver_cvodes.cpp +++ b/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(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 diff --git a/src/vector.cpp b/src/vector.cpp index 5d57dd9610..cefa789250 100644 --- a/src/vector.cpp +++ b/src/vector.cpp @@ -1,5 +1,7 @@ #include "amici/vector.h" +#include + namespace amici { AmiVector &AmiVector::operator=(AmiVector const &other) {