From eca64cc1ed466df98eab04c1396e9898b85240c5 Mon Sep 17 00:00:00 2001 From: dilpath Date: Wed, 16 Feb 2022 16:09:29 +0100 Subject: [PATCH 1/9] cache simplifications --- python/amici/ode_export.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index 3aa93e617e..217044f393 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -1036,7 +1036,18 @@ def __init__(self, verbose: Optional[Union[bool, int]] = False, } self._lock_total_derivative: List[str] = list() - self._simplify: Callable = simplify + self._simplify: Callable = None + if simplify is not None: + def cached_simplify( + expr: sp.Expr, + _simplified: Dict[str, sp.Expr] = {}, + _simplify: Callable = simplify, + ) -> sp.Expr: + expr_str = str(expr) + if expr_str not in _simplified: + _simplified[expr_str] = _simplify(expr) + return copy.deepcopy(_simplified[expr_str]) + self._simplify = cached_simplify self._x0_fixedParameters_idx: Union[None, Sequence[int]] self._w_recursion_depth: int = 0 self._has_quadratic_nllh: bool = True From ee7cd95e1cc81987170b46962ebb9d9dcf20666b Mon Sep 17 00:00:00 2001 From: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> Date: Wed, 16 Feb 2022 17:56:43 +0100 Subject: [PATCH 2/9] Update python/amici/ode_export.py Co-authored-by: Daniel Weindl --- python/amici/ode_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index 217044f393..244e14dc44 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -1043,7 +1043,7 @@ def cached_simplify( _simplified: Dict[str, sp.Expr] = {}, _simplify: Callable = simplify, ) -> sp.Expr: - expr_str = str(expr) + expr_str = repr(expr) if expr_str not in _simplified: _simplified[expr_str] = _simplify(expr) return copy.deepcopy(_simplified[expr_str]) From 768e26ab18f727c209e20ba04449954de544c275 Mon Sep 17 00:00:00 2001 From: dilpath Date: Wed, 16 Feb 2022 19:14:20 +0100 Subject: [PATCH 3/9] pysb error workaround: argument to enable caching; default sbml to cache; default pysb to not cache --- python/amici/ode_export.py | 11 ++++++++--- python/amici/pysb_import.py | 15 ++++++++++++++- python/amici/sbml_import.py | 10 +++++++++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index 244e14dc44..2940923185 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -959,7 +959,8 @@ class ODEModel: """ def __init__(self, verbose: Optional[Union[bool, int]] = False, - simplify: Optional[Callable] = sp.powsimp): + simplify: Optional[Callable] = sp.powsimp, + cache_simplify: bool = True): """ Create a new ODEModel instance. @@ -969,6 +970,10 @@ def __init__(self, verbose: Optional[Union[bool, int]] = False, :param simplify: see :meth:`ODEModel._simplify` + + :param cache_simplify: + Whether to cache calls to the simplify method. Can e.g. decrease + import times for models with events. """ self._states: List[State] = [] self._observables: List[Observable] = [] @@ -1036,8 +1041,8 @@ def __init__(self, verbose: Optional[Union[bool, int]] = False, } self._lock_total_derivative: List[str] = list() - self._simplify: Callable = None - if simplify is not None: + self._simplify: Callable = simplify + if cache_simplify and simplify is not None: def cached_simplify( expr: sp.Expr, _simplified: Dict[str, sp.Expr] = {}, diff --git a/python/amici/pysb_import.py b/python/amici/pysb_import.py index 85e0948fb7..1a732d58da 100644 --- a/python/amici/pysb_import.py +++ b/python/amici/pysb_import.py @@ -49,6 +49,7 @@ def pysb2amici( compute_conservation_laws: bool = True, compile: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), + cache_simplify: bool = False, generate_sensitivity_code: bool = True, ): r""" @@ -115,6 +116,9 @@ def pysb2amici( :param simplify: see :attr:`amici.ODEModel._simplify` + :param cache_simplify: + see :func:`amici.ODEModel.__init__` + :param generate_sensitivity_code: if set to ``False``, code for sensitivity computation will not be generated @@ -134,6 +138,7 @@ def pysb2amici( noise_distributions=noise_distributions, compute_conservation_laws=compute_conservation_laws, simplify=simplify, + cache_simplify=cache_simplify, verbose=verbose, ) exporter = ODEExporter( @@ -160,6 +165,7 @@ def ode_model_from_pysb_importer( noise_distributions: Optional[Dict[str, Union[str, Callable]]] = None, compute_conservation_laws: bool = True, simplify: Callable = sp.powsimp, + cache_simplify: bool = False, verbose: Union[int, bool] = False, ) -> ODEModel: """ @@ -188,6 +194,9 @@ def ode_model_from_pysb_importer( :param simplify: see :attr:`amici.ODEModel._simplify` + :param cache_simplify: + see :func:`amici.ODEModel.__init__` + :param verbose: verbosity level for logging, True/False default to :attr:`logging.DEBUG`/:attr:`logging.ERROR` @@ -195,7 +204,11 @@ def ode_model_from_pysb_importer( New ODEModel instance according to pysbModel """ - ode = ODEModel(verbose=verbose, simplify=simplify) + ode = ODEModel( + verbose=verbose, + simplify=simplify, + cache_simplify=cache_simplify, + ) if constant_parameters is None: constant_parameters = [] diff --git a/python/amici/sbml_import.py b/python/amici/sbml_import.py index b86e822beb..79524b9721 100644 --- a/python/amici/sbml_import.py +++ b/python/amici/sbml_import.py @@ -218,6 +218,7 @@ def sbml2amici(self, compile: bool = True, compute_conservation_laws: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), + cache_simplify: bool = True, log_as_log10: bool = True, generate_sensitivity_code: bool = True, **kwargs) -> None: @@ -288,6 +289,9 @@ def sbml2amici(self, :param simplify: see :attr:`ODEModel._simplify` + :param cache_simplify: + see :func:`amici.ODEModel.__init__` + :param log_as_log10: If ``True``, log in the SBML model will be parsed as ``log10`` (default), if ``False``, log will be parsed as natural logarithm @@ -361,7 +365,11 @@ def sbml2amici(self, self._clean_reserved_symbols() self._process_time() - ode_model = ODEModel(verbose=verbose, simplify=simplify) + ode_model = ODEModel( + verbose=verbose, + simplify=simplify, + cache_simplify=cache_simplify, + ) ode_model.import_from_sbml_importer( self, compute_cls=compute_conservation_laws) exporter = ODEExporter( From bd77cc700d92aaafbff9905a4694829060d06cff Mon Sep 17 00:00:00 2001 From: dilpath Date: Thu, 17 Feb 2022 16:15:25 +0100 Subject: [PATCH 4/9] remove copy --- python/amici/ode_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index 2940923185..d67bac4bd1 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -1051,7 +1051,7 @@ def cached_simplify( expr_str = repr(expr) if expr_str not in _simplified: _simplified[expr_str] = _simplify(expr) - return copy.deepcopy(_simplified[expr_str]) + return _simplified[expr_str] self._simplify = cached_simplify self._x0_fixedParameters_idx: Union[None, Sequence[int]] self._w_recursion_depth: int = 0 From 8c27c36cb66d735f82a2bad35a7092b629a00d69 Mon Sep 17 00:00:00 2001 From: dilpath Date: Thu, 17 Feb 2022 23:17:08 +0100 Subject: [PATCH 5/9] default no cache --- python/amici/sbml_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/amici/sbml_import.py b/python/amici/sbml_import.py index 0499d62cc1..49090d6475 100644 --- a/python/amici/sbml_import.py +++ b/python/amici/sbml_import.py @@ -218,7 +218,7 @@ def sbml2amici(self, compile: bool = True, compute_conservation_laws: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), - cache_simplify: bool = True, + cache_simplify: bool = False, log_as_log10: bool = True, generate_sensitivity_code: bool = True, **kwargs) -> None: From 96320ab945d32975457f7a50c783a9171dda271d Mon Sep 17 00:00:00 2001 From: dilpath Date: Thu, 17 Feb 2022 23:20:56 +0100 Subject: [PATCH 6/9] default off everywhere --- python/amici/ode_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index d67bac4bd1..7901624ce0 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -960,7 +960,7 @@ class ODEModel: def __init__(self, verbose: Optional[Union[bool, int]] = False, simplify: Optional[Callable] = sp.powsimp, - cache_simplify: bool = True): + cache_simplify: bool = False): """ Create a new ODEModel instance. From 8b574ca42d328daf6b007f67971447ee06ea9f50 Mon Sep 17 00:00:00 2001 From: dilpath Date: Fri, 18 Feb 2022 15:34:30 +0100 Subject: [PATCH 7/9] enable pysb again --- python/amici/pysb_import.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/amici/pysb_import.py b/python/amici/pysb_import.py index 1a732d58da..d40f769480 100644 --- a/python/amici/pysb_import.py +++ b/python/amici/pysb_import.py @@ -49,7 +49,7 @@ def pysb2amici( compute_conservation_laws: bool = True, compile: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), - cache_simplify: bool = False, + cache_simplify: bool = True, generate_sensitivity_code: bool = True, ): r""" @@ -165,7 +165,7 @@ def ode_model_from_pysb_importer( noise_distributions: Optional[Dict[str, Union[str, Callable]]] = None, compute_conservation_laws: bool = True, simplify: Callable = sp.powsimp, - cache_simplify: bool = False, + cache_simplify: bool = True, verbose: Union[int, bool] = False, ) -> ODEModel: """ From 5c98eeab71e24ae80936cece5a4f46a97d95c96a Mon Sep 17 00:00:00 2001 From: dilpath Date: Fri, 18 Feb 2022 16:40:54 +0100 Subject: [PATCH 8/9] exclude from pysb --- python/amici/pysb_import.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/python/amici/pysb_import.py b/python/amici/pysb_import.py index d40f769480..c8d439d0f3 100644 --- a/python/amici/pysb_import.py +++ b/python/amici/pysb_import.py @@ -49,7 +49,6 @@ def pysb2amici( compute_conservation_laws: bool = True, compile: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), - cache_simplify: bool = True, generate_sensitivity_code: bool = True, ): r""" @@ -116,9 +115,6 @@ def pysb2amici( :param simplify: see :attr:`amici.ODEModel._simplify` - :param cache_simplify: - see :func:`amici.ODEModel.__init__` - :param generate_sensitivity_code: if set to ``False``, code for sensitivity computation will not be generated @@ -138,7 +134,6 @@ def pysb2amici( noise_distributions=noise_distributions, compute_conservation_laws=compute_conservation_laws, simplify=simplify, - cache_simplify=cache_simplify, verbose=verbose, ) exporter = ODEExporter( @@ -165,7 +160,6 @@ def ode_model_from_pysb_importer( noise_distributions: Optional[Dict[str, Union[str, Callable]]] = None, compute_conservation_laws: bool = True, simplify: Callable = sp.powsimp, - cache_simplify: bool = True, verbose: Union[int, bool] = False, ) -> ODEModel: """ @@ -194,9 +188,6 @@ def ode_model_from_pysb_importer( :param simplify: see :attr:`amici.ODEModel._simplify` - :param cache_simplify: - see :func:`amici.ODEModel.__init__` - :param verbose: verbosity level for logging, True/False default to :attr:`logging.DEBUG`/:attr:`logging.ERROR` @@ -207,7 +198,6 @@ def ode_model_from_pysb_importer( ode = ODEModel( verbose=verbose, simplify=simplify, - cache_simplify=cache_simplify, ) if constant_parameters is None: From 8232cf8082becc808f5791ab8d4700cd193f6a00 Mon Sep 17 00:00:00 2001 From: dilpath Date: Fri, 18 Feb 2022 16:59:18 +0100 Subject: [PATCH 9/9] add usage warnings, add option to turn on for pysb --- python/amici/ode_export.py | 18 ++++++++++++++++++ python/amici/pysb_import.py | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/python/amici/ode_export.py b/python/amici/ode_export.py index 7901624ce0..4978b86a5c 100644 --- a/python/amici/ode_export.py +++ b/python/amici/ode_export.py @@ -1048,6 +1048,24 @@ def cached_simplify( _simplified: Dict[str, sp.Expr] = {}, _simplify: Callable = simplify, ) -> sp.Expr: + """Speed up expression simplification with caching. + + NB: This can decrease model import times for models that have + many repeated expressions during C++ file generation. + For example, this can be useful for models with events. + However, for other models, this may increase model import + times. + + :param expr: + The SymPy expression. + :param _simplified: + The cache. + :param _simplify: + The simplification method. + + :return: + The simplified expression. + """ expr_str = repr(expr) if expr_str not in _simplified: _simplified[expr_str] = _simplify(expr) diff --git a/python/amici/pysb_import.py b/python/amici/pysb_import.py index c8d439d0f3..bc7c15da0c 100644 --- a/python/amici/pysb_import.py +++ b/python/amici/pysb_import.py @@ -49,6 +49,9 @@ def pysb2amici( compute_conservation_laws: bool = True, compile: bool = True, simplify: Callable = lambda x: sp.powsimp(x, deep=True), + # Do not enable by default without testing. + # See https://github.com/AMICI-dev/AMICI/pull/1672 + cache_simplify: bool = False, generate_sensitivity_code: bool = True, ): r""" @@ -115,6 +118,11 @@ def pysb2amici( :param simplify: see :attr:`amici.ODEModel._simplify` + :param cache_simplify: + see :func:`amici.ODEModel.__init__` + Note that there are possible issues with PySB models: + https://github.com/AMICI-dev/AMICI/pull/1672 + :param generate_sensitivity_code: if set to ``False``, code for sensitivity computation will not be generated @@ -134,6 +142,7 @@ def pysb2amici( noise_distributions=noise_distributions, compute_conservation_laws=compute_conservation_laws, simplify=simplify, + cache_simplify=cache_simplify, verbose=verbose, ) exporter = ODEExporter( @@ -160,6 +169,9 @@ def ode_model_from_pysb_importer( noise_distributions: Optional[Dict[str, Union[str, Callable]]] = None, compute_conservation_laws: bool = True, simplify: Callable = sp.powsimp, + # Do not enable by default without testing. + # See https://github.com/AMICI-dev/AMICI/pull/1672 + cache_simplify: bool = False, verbose: Union[int, bool] = False, ) -> ODEModel: """ @@ -188,6 +200,11 @@ def ode_model_from_pysb_importer( :param simplify: see :attr:`amici.ODEModel._simplify` + :param cache_simplify: + see :func:`amici.ODEModel.__init__` + Note that there are possible issues with PySB models: + https://github.com/AMICI-dev/AMICI/pull/1672 + :param verbose: verbosity level for logging, True/False default to :attr:`logging.DEBUG`/:attr:`logging.ERROR` @@ -198,6 +215,7 @@ def ode_model_from_pysb_importer( ode = ODEModel( verbose=verbose, simplify=simplify, + cache_simplify=cache_simplify, ) if constant_parameters is None: