From 5ba5821a5f61ee6a3573bf199dc4997ad9915abc Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 18 Dec 2023 14:22:52 +0100 Subject: [PATCH 1/4] GHA: test python3.12 (#2179) Run nightly tests also on python3.12 --- .github/workflows/test_pypi.yml | 2 +- .github/workflows/test_python_ver_matrix.yml | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test_pypi.yml b/.github/workflows/test_pypi.yml index 4f3533850f..68675c578a 100644 --- a/.github/workflows/test_pypi.yml +++ b/.github/workflows/test_pypi.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.9", "3.10", "3.11"] + python-version: ["3.9", "3.10", "3.11", "3.12"] os: [ubuntu-22.04, macos-latest] runs-on: ${{ matrix.os }} diff --git a/.github/workflows/test_python_ver_matrix.yml b/.github/workflows/test_python_ver_matrix.yml index 866a3fc0f7..9290cd0c1a 100644 --- a/.github/workflows/test_python_ver_matrix.yml +++ b/.github/workflows/test_python_ver_matrix.yml @@ -25,7 +25,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.9', '3.10', '3.11'] + python-version: ['3.9', '3.10', '3.11', '3.12'] experimental: [false] steps: @@ -44,15 +44,27 @@ jobs: - name: Install apt dependencies uses: ./.github/actions/install-apt-dependencies - # install AMICI - name: Build BNGL run: scripts/buildBNGL.sh + - name: Install python package run: scripts/installAmiciSource.sh + # until https://github.com/dateutil/dateutil >2.8.2 is released https://github.com/dateutil/dateutil/issues/1314 + - run: source build/venv/bin/activate && pip3 install git+https://github.com/dateutil/dateutil.git@296d419fe6bf3b22897f8f210735ac9c4e1cb796 + if: matrix.python-version == '3.12' + + # install pysb before sympy to allow for sympy>=1.12 (https://github.com/pysb/pysb/commit/e83937cb8c74afc9b2fa96595b68464946745f33) + - run: source build/venv/bin/activate && pip3 install git+https://github.com/pysb/pysb + + # until sympy>1.12 is released + - run: source build/venv/bin/activate && pip3 install git+https://github.com/sympy/sympy.git@master + if: matrix.python-version == '3.12' + - name: Python tests run: | source build/venv/bin/activate \ - && pip3 install git+https://github.com/pysb/pysb \ - && python3 -m pytest --ignore-glob=*petab* \ + && python3 -m pytest \ + --durations=10 \ + --ignore-glob=*petab* \ --ignore-glob=*test_splines.py ${AMICI_DIR}/python/tests From b61c0308975d21d77e645bbe905c92f0f736755c Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 18 Dec 2023 14:25:31 +0100 Subject: [PATCH 2/4] Deterministic order of event assignments (#2242) Ensure event assignments targets are processed in deterministic order. Otherwise the ordering of state variables may change between subsequent model imports, which we'd like to avoid. Closes #2241. --- python/sdist/amici/sbml_import.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/sdist/amici/sbml_import.py b/python/sdist/amici/sbml_import.py index b6a7b37ed4..124dfc7a63 100644 --- a/python/sdist/amici/sbml_import.py +++ b/python/sdist/amici/sbml_import.py @@ -2776,15 +2776,17 @@ def replace_logx(math_str: Union[str, float, None]) -> Union[str, float, None]: return re.sub(r"(^|\W)log(\d+)\(", r"\g<1>1/ln(\2)*ln(", math_str) -def _collect_event_assignment_parameter_targets(sbml_model: sbml.Model): - targets = set() +def _collect_event_assignment_parameter_targets( + sbml_model: sbml.Model, +) -> list[sp.Symbol]: + targets = [] sbml_parameters = sbml_model.getListOfParameters() sbml_parameter_ids = [p.getId() for p in sbml_parameters] for event in sbml_model.getListOfEvents(): for event_assignment in event.getListOfEventAssignments(): target_id = event_assignment.getVariable() if target_id in sbml_parameter_ids: - targets.add( + targets.append( _get_identifier_symbol( sbml_parameters[sbml_parameter_ids.index(target_id)] ) From e213d593a800550e15beb556285e5c021b9ece00 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 18 Dec 2023 14:26:53 +0100 Subject: [PATCH 3/4] Fix AMICI hiding all warnings (#2243) Previously, importing amici would result in all warnings of the program being hidden, due to `logging.captureWarnings(True)`: ```sh $ python -c "import warnings; warnings.warn('bla');" :1: UserWarning: bla $ python -c "import amici; import warnings; warnings.warn('bla');" $ ``` This can't be the desired default. Changes: * Default to not capturing warnings * If warnings are to be captured, at least handle them by amici loggers Closes ICB-DCM/pyPESTO#1252 --- python/sdist/amici/logging.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/python/sdist/amici/logging.py b/python/sdist/amici/logging.py index 2648fc5b28..df39c4a219 100644 --- a/python/sdist/amici/logging.py +++ b/python/sdist/amici/logging.py @@ -34,24 +34,24 @@ def _setup_logger( level: Optional[int] = logging.WARNING, console_output: Optional[bool] = True, file_output: Optional[bool] = False, - capture_warnings: Optional[bool] = True, + capture_warnings: Optional[bool] = False, ) -> logging.Logger: """ - Set up a new logging.Logger for AMICI logging + Set up a new :class:`logging.Logger` for AMICI logging. :param level: - Logging level, typically using a constant like logging.INFO or - logging.DEBUG + Logging level, typically using a constant like :obj:`logging.INFO` or + :obj:`logging.DEBUG` :param console_output: - Set up a default console log handler if True (default) + Set up a default console log handler if ``True`` (default) :param file_output: Supply a filename to copy all log output to that file, or - set to False to disable (default) + set to ``False`` to disable (default) :param capture_warnings: - Capture warnings from Python's warnings module if True (default) + Capture warnings from Python's warnings module if ``True`` :return: A :class:`logging.Logger` object for AMICI logging. Note that other @@ -81,7 +81,12 @@ def _setup_logger( log.setLevel(level) + py_warn_logger = logging.getLogger("py.warnings") + # Remove default logging handler + for handler in log.handlers: + if handler in py_warn_logger.handlers: + py_warn_logger.removeHandler(handler) log.handlers = [] log_fmt = logging.Formatter( @@ -105,7 +110,10 @@ def _setup_logger( log.debug("Python version: %s", platform.python_version()) log.debug("Hostname: %s", socket.getfqdn()) - logging.captureWarnings(capture_warnings) + if capture_warnings: + logging.captureWarnings(capture_warnings) + for handler in log.handlers: + py_warn_logger.addHandler(handler) return log From ba4f8565f3ed9c495c6a39be2c103be194e3355c Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 18 Dec 2023 14:28:30 +0100 Subject: [PATCH 4/4] Make solvers and SolverPtr deepcopyable (#2245) Make solvers and SolverPtr deepcopyable, and fix a segfault when creating `SolverPtr`s from Python. --- python/tests/test_swig_interface.py | 16 ++++++++++++++++ swig/solver.i | 9 +++++++++ swig/std_unique_ptr.i | 3 +++ 3 files changed, 28 insertions(+) diff --git a/python/tests/test_swig_interface.py b/python/tests/test_swig_interface.py index 6a7cfec855..e8dd478cab 100644 --- a/python/tests/test_swig_interface.py +++ b/python/tests/test_swig_interface.py @@ -471,3 +471,19 @@ def test_expdata_and_expdataview_are_deepcopyable(): ev2 = copy.deepcopy(ev1) assert ev2._swigptr.this != ev1._swigptr.this assert ev1 == ev2 + + +def test_solvers_are_deepcopyable(): + for solver_type in (amici.CVodeSolver, amici.IDASolver): + for solver1 in (solver_type(), amici.SolverPtr(solver_type())): + solver2 = copy.deepcopy(solver1) + assert solver1.this != solver2.this + assert ( + solver1.getRelativeTolerance() + == solver2.getRelativeTolerance() + ) + solver2.setRelativeTolerance(100 * solver2.getRelativeTolerance()) + assert ( + solver1.getRelativeTolerance() + != solver2.getRelativeTolerance() + ) diff --git a/swig/solver.i b/swig/solver.i index 992842c409..20641ba31f 100644 --- a/swig/solver.i +++ b/swig/solver.i @@ -113,9 +113,18 @@ def __repr__(self): %pythoncode %{ def __repr__(self): return _solver_repr(self) + +def __deepcopy__(self, memo): + return self.clone() %} }; +%extend amici::Solver { +%pythoncode %{ +def __deepcopy__(self, memo): + return self.clone() +%} +}; %newobject amici::Solver::clone; // Process symbols in header diff --git a/swig/std_unique_ptr.i b/swig/std_unique_ptr.i index 1063bd75b1..c44513bcee 100644 --- a/swig/std_unique_ptr.i +++ b/swig/std_unique_ptr.i @@ -6,8 +6,11 @@ namespace std { struct unique_ptr { typedef Type* pointer; + %apply SWIGTYPE *DISOWN { pointer Ptr }; explicit unique_ptr( pointer Ptr ); + %clear pointer Ptr; unique_ptr (unique_ptr&& Right); + template unique_ptr( unique_ptr&& Right ); unique_ptr( const unique_ptr& Right) = delete;