Merge pull request #501 from ICB-DCM/release_v0.8.2

Release v0.8.2 Features / improvements: * Speedup symbolic processing for ODE generation in python Bugfixes: * Fix(python) Add missing deepcopy (introduced in 6847ba6) * Fix(core) Prevent parameter scaling length mismatch (#488) * Fix(python) Set distutils dependency to current version to fix </usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'> * fix(python) add symlink to version.txt to be included in package Backwards-compatibility: * Replace 'newline' by literal to restore <R2016b compatibility (Fixes #493) Maintenance: * Remove obsolete swig library build via cmake and related file copying * Provide issue template for bug reports * Providing valid SBML document to import is not optional anymore * Update documentation and tests * Add python version check and raise required version to 3.6 to prevent cryptic error messages when encountering f-strings
AMICI-dev · Jan 7, 2019 · dce9bbc · dce9bbc
2 parents c6a3345 + a4d703d
commit dce9bbc
Show file tree

Hide file tree

Showing 32 changed files with 225 additions and 364 deletions.
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,26 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: new
+assignees: ''
+
+---
+
+**What did you expect to happen?**
+
+**What has happened instead?**
+
+**To Reproduce**
+*Steps to reproduce the behavior*
+*Ideally include minimal code examples here*
+
+**AMICI version and system environment**
+ - OS and version: [e.g. Ubuntu, iOS, Windows]
+ - AMICI interface: [e.g. Python, Matlab, C++]
+ - AMICI version:
+ - Additional information: Compiler name and version used, Python/Matlab version, ...
+
+**How to fix**
+Do you know how to resolve the problem?
+Can you submit a pull request?
diff --git a/.travis.yml b/.travis.yml
@@ -4,7 +4,7 @@ sudo: required
 branches:
   only:
     - master
-    - staging
+    - develop
 
 matrix:
   fast_finish: true

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -172,9 +172,9 @@ endif()
 
 # build interfaces for other languages
 option(ENABLE_SWIG "Build AMICI swig library?" ON)
-#if(ENABLE_SWIG)
-#    add_subdirectory(swig)
-#endif()
+if(ENABLE_SWIG)
+    add_subdirectory(swig)
+endif()
 
 option(ENABLE_PYTHON "Create Python module?" ON)
 if(ENABLE_PYTHON)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -7,17 +7,18 @@ We are happy about contributions to AMICI in any form (new functionality, docume
 When making code changes:
 
 * Check if you agree to release your contribution under the conditions provided in `LICENSE`
-* Start a new branch from `master`
+* Start a new branch from `develop`
 * Implement your changes
-* Submit a pull request
+* Submit a pull request to the `develop` branch
 * Make sure your code is documented appropriately
   * Run `mtoc/makeDocumentation.m` to check completeness of your documentation
 * Make sure your code is compatible with C++11, `gcc` and `clang`
 * when adding new functionality, please also provide test cases (see `tests/cpputest/`)
 * Write meaningful commit messages
-* Run all tests to ensure nothing got broken
+* Run all tests to ensure nothing was broken
   * Run `tests/cpputest/wrapTestModels.m` followed by CI tests `scripts/buildAll.sh && scripts/run-cpputest.sh`
   * Run `tests/testModels.m`
+  * Run `make python-tests` in `build`
 * When all tests are passing and you think your code is ready to merge, request a code review
 
 ## Adding/Updating tests

diff --git a/matlab/@amimodel/compileAndLinkModel.m b/matlab/@amimodel/compileAndLinkModel.m
@@ -331,7 +331,7 @@ function updateFileHash(fileFolder,hashFolder,filename)
 
 function versionstring = getCompilerVersionString()
     [~,systemreturn] = system([mex.getCompilerConfigurations('c++').Details.CompilerExecutable ' --version']);
-    newlinePos = strfind(systemreturn,newline);
+    newlinePos = strfind(systemreturn, sprintf('\n'));
     str = systemreturn(1:(newlinePos(1)-1));
     str = regexprep(str,'[\(\)]','');
     str = regexprep(str,'[\s\.\-]','_');

diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
@@ -1,41 +1,9 @@
 find_package(PythonInterp 3.6 REQUIRED)
 
-# Configure setup.py and copy to output directory
-set(AMICI_VERSION "@AMICI_VERSION@") # to be replaced later
-configure_file(${PROJECT_SOURCE_DIR}/python/amici/setup.template.py ${CMAKE_CURRENT_BINARY_DIR}/setup.template.py)
-
-# Copy further amici-python-module files to binary_dir
-add_custom_target(python-module-files
-    DEPENDS always_rebuild
-    COMMENT "Preparing python module directory"
-    COMMAND ${CMAKE_COMMAND} -D SRC=${CMAKE_CURRENT_BINARY_DIR}/setup.py
-                             -D DST=${CMAKE_CURRENT_BINARY_DIR}/setup.py
-                                -P ${PROJECT_SOURCE_DIR}/cmake/configureVersion.cmake
-    COMMAND ${CMAKE_COMMAND} -E remove_directory ${CMAKE_CURRENT_BINARY_DIR}/amici
-    COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/amici
-    COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/python/amici ${CMAKE_CURRENT_BINARY_DIR}/amici
-    COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/src ${CMAKE_CURRENT_BINARY_DIR}/amici/src
-    COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/include ${CMAKE_CURRENT_BINARY_DIR}/amici/include
-    COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/swig ${CMAKE_CURRENT_BINARY_DIR}/amici/swig
-    COMMAND ${CMAKE_COMMAND} -E copy_directory ${PROJECT_SOURCE_DIR}/ThirdParty ${CMAKE_CURRENT_BINARY_DIR}/amici/ThirdParty
-    COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/setup.template.py ${CMAKE_CURRENT_BINARY_DIR}/amici/setup.template.py
-    COMMAND ${CMAKE_COMMAND} -E copy ${PROJECT_SOURCE_DIR}/README.md ${CMAKE_CURRENT_BINARY_DIR}/
-    COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_BINARY_DIR}/libamici.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/sundials/build/lib/libsundials_nvecserial.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/sundials/build/lib/libsundials_cvodes.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/sundials/build/lib/libsundials_idas.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/SuiteSparse/KLU/Lib/libklu.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/SuiteSparse/COLAMD/Lib/libcolamd.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/SuiteSparse/BTF/Lib/libbtf.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/SuiteSparse/AMD/Lib/libamd.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    COMMAND ${CMAKE_COMMAND} -E copy "${PROJECT_SOURCE_DIR}/ThirdParty/SuiteSparse/SuiteSparse_config/libsuitesparseconfig.a" ${CMAKE_CURRENT_BINARY_DIR}/amici/libs/
-    )
-
 add_custom_target(install-python
     COMMENT "Installing AMICI base python package"
-    DEPENDS python-module-files
-    COMMAND ${PYTHON_EXECUTABLE} ${SETUP_PY_OUT} install --prefix= --user)
+    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/sdist
+    COMMAND ${PYTHON_EXECUTABLE} setup.py install --prefix= --user)
 
 
 # Create python wheel
@@ -44,19 +12,20 @@ add_custom_target(install-python
 # build_py stage
 add_custom_target(python-wheel
     COMMENT "Creating wheel for AMICI base python package"
-    DEPENDS python-module-files
+    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/sdist
     COMMAND ${PYTHON_EXECUTABLE} setup.py build_ext
     COMMAND ${PYTHON_EXECUTABLE} setup.py bdist_wheel --dist-dir=${CMAKE_CURRENT_BINARY_DIR}
 )
 
+
 add_custom_target(python-sdist
     COMMENT "Creating sdist for AMICI base python package"
-    COMMAND ${PYTHON_EXECUTABLE} setup.py sdist --dist-dir=${CMAKE_CURRENT_BINARY_DIR}
     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/sdist
+    COMMAND ${PYTHON_EXECUTABLE} setup.py sdist --dist-dir=${CMAKE_CURRENT_BINARY_DIR}
 )
 
+
 add_custom_command(
     OUTPUT always_rebuild
     COMMAND cmake -E echo
     )
-
diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py
@@ -1109,7 +1109,7 @@ def _computeEquation(self, name):
             dx0_fixedParametersdx = \
                 self.eq('x0_fixedParameters').jacobian(self.sym('x'))
 
-            if not dx0_fixedParametersdx.is_zero:
+            if dx0_fixedParametersdx.is_zero is not True:
                 for ip in range(self._eqs[name].shape[1]):
                     self._eqs[name][:,ip] += \
                         dx0_fixedParametersdx \
@@ -1206,8 +1206,11 @@ def _derivative(self, eq, var, name=None):
         else:
             eq = self.eq(eq)
 
-        if min(eq.shape) and min(self.sym(var).shape):
-            self._eqs[name] = eq.jacobian(self.sym(var))
+        sym_var = self.sym(var)
+
+        if min(eq.shape) and min(sym_var.shape) \
+                and eq.is_zero is not True and sym_var.is_zero is not True:
+            self._eqs[name] = eq.jacobian(sym_var)
         else:
             self._eqs[name] = sp.zeros(eq.shape[0], self.sym(var).shape[0])
 
@@ -1264,10 +1267,14 @@ def _totalDerivative(self, name, eq, chainvar, var,
                 variables[var]['sym'] = self.eq(varname)
 
         self._eqs[name] = \
-            variables['dydz']['sym'] \
-            + variables['dydx']['sym'] * variables['dxdz']['sym']
-
+            copy.deepcopy(variables['dydz']['sym'])
 
+        # Save time for for large models if one multiplicand is zero,
+        # which is not checked for by sympy
+        if variables['dydx']['sym'].is_zero is not True \
+                and variables['dxdz']['sym'].is_zero is not True:
+            self._eqs[name] += variables['dydx']['sym'] * \
+                               variables['dxdz']['sym']
 
 
     def _multiplication(self, name, x, y,
@@ -1617,6 +1624,10 @@ def _writeFunctionFile(self, function):
         if 'sparse' in self.functions[function] and \
                 self.functions[function]['sparse']:
             symbol = self.model.sparseeq(function)
+        elif not self.allow_reinit_fixpar_initcond \
+                and function == 'sx0_fixedParameters':
+            # Not required. Will create empty function body.
+            symbol = sp.Matrix()
         else:
             symbol = self.model.eq(function)
 

diff --git a/python/amici/sbml_import.py b/python/amici/sbml_import.py
@@ -1,6 +1,6 @@
+#!/usr/bin/env python3
 """ @package amici.sbml_import The python sbml import module for python
 """
-#!/usr/bin/env python3
 
 import sympy as sp
 import libsbml as sbml
@@ -31,15 +31,15 @@ class SbmlImporter:
 
     Attributes:
 
-        check_validity: indicates whether the validity of the SBML document
-        should be checked @type bool
+        show_sbml_warnings: indicates whether libSBML warnings should be
+        displayed @type bool
 
         symbols: dict carrying symbolic definitions @type dict
 
         SBMLreader: the libSBML sbml reader [!not storing this will result
         in a segfault!]
 
-        sbml_doc: document carrying the sbml defintion [!not storing this
+        sbml_doc: document carrying the sbml definition [!not storing this
         will result in a segfault!]
 
         sbml: sbml definition [!not storing this will result in a segfault!]
@@ -62,25 +62,25 @@ class SbmlImporter:
 
         compartmentSymbols: compartment ids @type sympy.Matrix
 
-        compartmentVolume: numeric/symbnolic compartment volumes @type
+        compartmentVolume: numeric/symbolic compartment volumes @type
         sympy.Matrix
 
-        stoichiometricMatrix: stoichiometrix matrix of the model @type
+        stoichiometricMatrix: stoichiometric matrix of the model @type
         sympy.Matrix
 
         fluxVector: reaction kinetic laws @type sympy.Matrix
 
     """
 
-    def __init__(self, SBMLFile, check_validity=True):
+    def __init__(self, SBMLFile, show_sbml_warnings=False):
         """Create a new Model instance.
 
         Arguments:
 
         SBMLFile: Path to SBML file where the model is specified @type string
 
-        check_validity: Flag indicating whether the validity of the SBML
-        document should be checked @type bool
+        show_sbml_warnings: indicates whether libSBML warnings should be
+        displayed @type bool
 
         Returns:
         SbmlImporter instance with attached SBML document
@@ -89,7 +89,7 @@ def __init__(self, SBMLFile, check_validity=True):
 
         """
 
-        self.check_validity = check_validity
+        self.show_sbml_warnings = show_sbml_warnings
 
         self.loadSBMLFile(SBMLFile)
 
@@ -123,10 +123,11 @@ def loadSBMLFile(self, SBMLFile):
 
         self.SBMLreader = sbml.SBMLReader()
         self.sbml_doc = self.SBMLreader.readSBML(SBMLFile)
-        self.checkLibSBMLErrors()
-        # If any of the above calls produces an error, this will be added to
-        # the SBMLError log in the sbml document. Thus, it is sufficient to
-        # check the error log just once after all conversion/validation calls.
+
+        # Ensure we got a valid SBML model, otherwise further processing
+        # might lead to undefined results
+        self.sbml_doc.validateSBML()
+        checkLibSBMLErrors(self.sbml_doc, self.show_sbml_warnings)
 
         # apply several model simplifications that make our life substantially
         # easier
@@ -139,45 +140,13 @@ def loadSBMLFile(self, SBMLFile):
             getDefaultProperties()
         self.sbml_doc.convert(convertConfig)
 
-        if self.check_validity:
-            self.sbml_doc.validateSBML()
-
         # If any of the above calls produces an error, this will be added to
         # the SBMLError log in the sbml document. Thus, it is sufficient to
         # check the error log just once after all conversion/validation calls.
-        self.checkLibSBMLErrors()
+        checkLibSBMLErrors(self.sbml_doc, self.show_sbml_warnings)
 
         self.sbml = self.sbml_doc.getModel()
 
-    def checkLibSBMLErrors(self):
-        """Checks the error log in the current self.sbml_doc
-
-        Arguments:
-
-        Returns:
-
-        Raises:
-        raises SBMLException if errors with severity ERROR or FATAL have
-        occured
-
-        """
-        num_warning = self.sbml_doc.getNumErrors(sbml.LIBSBML_SEV_WARNING)
-        num_error = self.sbml_doc.getNumErrors(sbml.LIBSBML_SEV_ERROR)
-        num_fatal = self.sbml_doc.getNumErrors(sbml.LIBSBML_SEV_FATAL)
-        if num_warning + num_error + num_fatal:
-            for iError in range(0, self.sbml_doc.getNumErrors()):
-                error = self.sbml_doc.getError(iError)
-                # we ignore any info messages for now
-                if error.getSeverity() >= sbml.LIBSBML_SEV_WARNING:
-                    category = error.getCategoryAsString()
-                    severity = error.getSeverityAsString()
-                    error_message = error.getMessage()
-                    print(f'libSBML {severity} ({category}): {error_message}')
-        if num_error + num_fatal:
-            raise SBMLException(
-                'SBML Document failed to load (see error messages above)'
-            )
-
     def sbml2amici(self,
                    modelName,
                    output_dir=None,
@@ -766,11 +735,13 @@ def processObservables(self, observables, sigmas):
 
         if sigmas is None:
             sigmas = {}
-        elif len(set(sigmas.keys()) - set(observables.keys())):
-            # Ensure no non-existing observableIds have been specified (no problem here, but usually an upstream bug)
-            raise ValueError(f'Sigma provided for an unknown observableId: {set(sigmas.keys()) - set(observables.keys())}')
-
-
+        else:
+            # Ensure no non-existing observableIds have been specified
+            # (no problem here, but usually an upstream bug)
+            unknown_observables = set(sigmas.keys()) - set(observables.keys())
+            if unknown_observables:
+                raise ValueError('Sigma provided for an unknown observableId: '
+                                 + str(unknown_observables))
 
         speciesSyms = self.symbols['species']['identifier']
 
@@ -1101,3 +1072,38 @@ def l2s(inputs):
 
     """
     return [str(inp) for inp in inputs]
+
+
+def checkLibSBMLErrors(sbml_doc, show_warnings=False):
+    """Checks the error log in the current self.sbml_doc
+
+    Arguments:
+        sbml_doc: SBML document @type libsbml.SBMLDocument
+        show_warnings: display SBML warnings @type bool
+
+    Returns:
+
+    Raises:
+        raises SBMLException if errors with severity ERROR or FATAL have
+        occurred
+    """
+    num_warning = sbml_doc.getNumErrors(sbml.LIBSBML_SEV_WARNING)
+    num_error = sbml_doc.getNumErrors(sbml.LIBSBML_SEV_ERROR)
+    num_fatal = sbml_doc.getNumErrors(sbml.LIBSBML_SEV_FATAL)
+
+    if num_warning + num_error + num_fatal:
+        for iError in range(0, sbml_doc.getNumErrors()):
+            error = sbml_doc.getError(iError)
+            # we ignore any info messages for now
+            if error.getSeverity() >= sbml.LIBSBML_SEV_ERROR \
+                    or (show_warnings and
+                        error.getSeverity() >= sbml.LIBSBML_SEV_WARNING):
+                category = error.getCategoryAsString()
+                severity = error.getSeverityAsString()
+                error_message = error.getMessage()
+                print(f'libSBML {severity} ({category}): {error_message}')
+
+    if num_error + num_fatal:
+        raise SBMLException(
+            'SBML Document failed to load (see error messages above)'
+        )