From 6b26c69a077298043093b67fbb3263bf0dd5423f Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Tue, 26 Apr 2022 22:57:01 +0200 Subject: [PATCH 01/28] Add abstraction for models --- petab/models/model.py | 50 +++++++++++++++++++++++++++++++++ petab/models/sbml_model.py | 57 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 petab/models/model.py create mode 100644 petab/models/sbml_model.py diff --git a/petab/models/model.py b/petab/models/model.py new file mode 100644 index 00000000..ab3c33aa --- /dev/null +++ b/petab/models/model.py @@ -0,0 +1,50 @@ +"""PEtab model abstraction""" +from __future__ import annotations + +import abc +from typing import Any, Iterable, Tuple + + +class Model: + """Base class for wrappers for any PEtab-supported model type""" + + def __init__(self): + ... + + @staticmethod + @abc.abstractmethod + def from_file(filepath_or_buffer: Any) -> Model: + ... + + @abc.abstractmethod + def get_parameter_ids(self) -> Iterable[str]: + ... + + @abc.abstractmethod + def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: + ... + + @abc.abstractmethod + def has_species_with_id(self, entity_id: str) -> bool: + ... + + @abc.abstractmethod + def has_compartment_with_id(self, entity_id: str) -> bool: + ... + + @abc.abstractmethod + def has_entity_with_id(self, entity_id) -> bool: + ... + + @abc.abstractmethod + def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: + ... + + +def model_factory(filepath_or_buffer: Any, model_language: str) -> Model: + """Create a PEtab model instance from the given model""" + if model_language == "sbml": + from .sbml_model import SbmlModel + return SbmlModel.from_file(filepath_or_buffer) + + raise ValueError(f"Unsupported model format: {model_language}") diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py new file mode 100644 index 00000000..87902fa7 --- /dev/null +++ b/petab/models/sbml_model.py @@ -0,0 +1,57 @@ +"""Functions for handling SBML models""" + +from .model import Model +from ..sbml import get_sbml_model +import libsbml +from typing import Optional, Iterable, Tuple + + +class SbmlModel(Model): + """PEtab wrapper for SBML models""" + def __init__( + self, + sbml_model: libsbml.Model = None, + sbml_reader: libsbml.SBMLReader = None, + sbml_document: libsbml.SBMLDocument = None, + ): + super().__init__() + + self.sbml_reader: Optional[libsbml.SBMLReader] = sbml_reader + self.sbml_document: Optional[libsbml.SBMLDocument] = sbml_document + self.sbml_model: Optional[libsbml.Model] = sbml_model + + @staticmethod + def from_file(filepath_or_buffer): + sbml_reader, sbml_document, sbml_model = get_sbml_model( + filepath_or_buffer) + return SbmlModel( + sbml_model=sbml_model, + sbml_reader=sbml_reader, + sbml_document=sbml_document, + ) + + def get_parameter_ids(self) -> Iterable[str]: + return ( + p.getId() for p in self.sbml_model.getListOfParameters() + if self.sbml_model.getAssignmentRuleByVariable(p.getId()) is None + ) + + def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: + return ( + (p.getId(), p.getValue()) + for p in self.sbml_model.getListOfParameters() + if self.sbml_model.getAssignmentRuleByVariable(p.getId()) is None + ) + + def has_species_with_id(self, entity_id: str) -> bool: + return self.sbml_model.getSpecies(entity_id) is not None + + def has_compartment_with_id(self, entity_id: str) -> bool: + return self.sbml_model.getCompartment(entity_id) is not None + + def has_entity_with_id(self, entity_id) -> bool: + return self.sbml_model.getElementBySId(entity_id) is not None + + def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: + # TODO + pass From 796d04285f4976804baa67186103a2e2b2bc93a5 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 27 Apr 2022 17:57:38 +0200 Subject: [PATCH 02/28] Use new Model in petab.Problem --- petab/models/sbml_model.py | 28 +++++++++++++++- petab/problem.py | 66 +++++++++++++++++++++----------------- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 87902fa7..cd5a19d8 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -1,7 +1,7 @@ """Functions for handling SBML models""" from .model import Model -from ..sbml import get_sbml_model +from ..sbml import get_sbml_model, load_sbml_from_string import libsbml from typing import Optional, Iterable, Tuple @@ -20,6 +20,32 @@ def __init__( self.sbml_document: Optional[libsbml.SBMLDocument] = sbml_document self.sbml_model: Optional[libsbml.Model] = sbml_model + def __getstate__(self): + """Return state for pickling""" + state = self.__dict__.copy() + + # libsbml stuff cannot be serialized directly + if self.sbml_model: + sbml_document = self.sbml_model.getSBMLDocument() + sbml_writer = libsbml.SBMLWriter() + state['sbml_string'] = sbml_writer.writeSBMLToString(sbml_document) + + exclude = ['sbml_reader', 'sbml_document', 'sbml_model'] + for key in exclude: + state.pop(key) + + return state + + def __setstate__(self, state): + """Set state after unpickling""" + # load SBML model from pickled string + sbml_string = state.pop('sbml_string', None) + if sbml_string: + self.sbml_reader, self.sbml_document, self.sbml_model = \ + load_sbml_from_string(sbml_string) + + self.__dict__.update(state) + @staticmethod def from_file(filepath_or_buffer): sbml_reader, sbml_document, sbml_model = get_sbml_model( diff --git a/petab/problem.py b/petab/problem.py index 517b91c9..39b18d59 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -12,6 +12,8 @@ from . import (conditions, core, format_version, measurements, observables, parameter_mapping, parameters, sampling, sbml, yaml) from .C import * # noqa: F403 +from .models.model import Model +from .models.sbml_model import SbmlModel __all__ = ['Problem'] @@ -43,6 +45,7 @@ def __init__(self, sbml_model: libsbml.Model = None, sbml_reader: libsbml.SBMLReader = None, sbml_document: libsbml.SBMLDocument = None, + model: Model = None, condition_df: pd.DataFrame = None, measurement_df: pd.DataFrame = None, parameter_df: pd.DataFrame = None, @@ -55,35 +58,40 @@ def __init__(self, self.visualization_df: Optional[pd.DataFrame] = visualization_df self.observable_df: Optional[pd.DataFrame] = observable_df - self.sbml_reader: Optional[libsbml.SBMLReader] = sbml_reader - self.sbml_document: Optional[libsbml.SBMLDocument] = sbml_document - self.sbml_model: Optional[libsbml.Model] = sbml_model - - def __getstate__(self): - """Return state for pickling""" - state = self.__dict__.copy() - - # libsbml stuff cannot be serialized directly - if self.sbml_model: - sbml_document = self.sbml_model.getSBMLDocument() - sbml_writer = libsbml.SBMLWriter() - state['sbml_string'] = sbml_writer.writeSBMLToString(sbml_document) - - exclude = ['sbml_reader', 'sbml_document', 'sbml_model'] - for key in exclude: - state.pop(key) - - return state - - def __setstate__(self, state): - """Set state after unpickling""" - # load SBML model from pickled string - sbml_string = state.pop('sbml_string', None) - if sbml_string: - self.sbml_reader, self.sbml_document, self.sbml_model = \ - sbml.load_sbml_from_string(sbml_string) - - self.__dict__.update(state) + if any((sbml_model, sbml_document, sbml_reader),): + warn("Passing `sbml_model`, `sbml_document`, or `sbml_reader` " + "to petab.Problem is deprecated and will be removed in a " + "future version. Use `model=petab.models.SbmlModel(...)` " + "instead.", DeprecationWarning, stacklevel=2) + if model: + raise ValueError("Must only provide one of (`sbml_model`, " + "`sbml_document`, `sbml_reader`) or `model`.") + + model = SbmlModel( + sbml_model=sbml_model, + sbml_reader=sbml_reader, + sbml_document=sbml_document) + + self.model: Optional[Model] = model + + def __getattr__(self, name): + # For backward-compatibility, allow access to SBML model related + # attributes now stored in self.model + if name in {'sbml_model', 'sbml_reader', 'sbml_document'}: + return getattr(self.model, name) if self.model else None + raise AttributeError(f"'{self.__class__.__name__}' object has no " + f"attribute '{name}'") + + def __setattr__(self, name, value): + # For backward-compatibility, allow access to SBML model related + # attributes now stored in self.model + if name in {'sbml_model', 'sbml_reader', 'sbml_document'}: + if self.model: + setattr(self.model, name, value) + else: + self.model = SbmlModel(**{name: value}) + else: + super().__setattr__(name, value) @staticmethod def from_files( From 796219bcb9a889281376276c0b86f7ccfba552ce Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 27 Apr 2022 23:42:34 +0200 Subject: [PATCH 03/28] __init__.py --- petab/models/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 petab/models/__init__.py diff --git a/petab/models/__init__.py b/petab/models/__init__.py new file mode 100644 index 00000000..e69de29b From d2003968ea8d1b08ad6194eddc9b2b26fb09d2e1 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 28 Apr 2022 16:27:48 +0200 Subject: [PATCH 04/28] fix merge --- doc/example/example_visualization.ipynb | 2 +- doc/example/example_visualization_without_visspec.ipynb | 2 +- petab/problem.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/example/example_visualization.ipynb b/doc/example/example_visualization.ipynb index 17118e31..7a55bf48 100644 --- a/doc/example/example_visualization.ipynb +++ b/doc/example/example_visualization.ipynb @@ -264,4 +264,4 @@ }, "nbformat": 4, "nbformat_minor": 2 -} +} \ No newline at end of file diff --git a/doc/example/example_visualization_without_visspec.ipynb b/doc/example/example_visualization_without_visspec.ipynb index abd53cc7..6200b928 100644 --- a/doc/example/example_visualization_without_visspec.ipynb +++ b/doc/example/example_visualization_without_visspec.ipynb @@ -168,4 +168,4 @@ }, "nbformat": 4, "nbformat_minor": 2 -} +} \ No newline at end of file diff --git a/petab/problem.py b/petab/problem.py index 39b18d59..a562c759 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -5,6 +5,7 @@ from pathlib import Path, PurePosixPath from typing import Dict, Iterable, List, Optional, Union from urllib.parse import unquote, urlparse, urlunparse +from warnings import warn import libsbml import pandas as pd From 52f0ec04d3f0286cc98b0c3af91ada067a636c31 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 28 Apr 2022 17:07:04 +0200 Subject: [PATCH 05/28] TYPE_CHECKING --- petab/problem.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/petab/problem.py b/petab/problem.py index a562c759..23579f0d 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -1,13 +1,13 @@ """PEtab Problem class""" +from __future__ import annotations import os import tempfile from pathlib import Path, PurePosixPath -from typing import Dict, Iterable, List, Optional, Union +from typing import Dict, Iterable, List, Optional, Union, TYPE_CHECKING from urllib.parse import unquote, urlparse, urlunparse from warnings import warn -import libsbml import pandas as pd from . import (conditions, core, format_version, measurements, observables, @@ -16,6 +16,10 @@ from .models.model import Model from .models.sbml_model import SbmlModel +if TYPE_CHECKING: + import libsbml + + __all__ = ['Problem'] From 0736225858349d3416693449eec72b4faffe78ff Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Fri, 29 Apr 2022 13:02:00 +0200 Subject: [PATCH 06/28] Replace most sbml_model occurrences by new Model --- petab/lint.py | 89 ++++++++++++++++++------------------- petab/models/__init__.py | 1 + petab/models/model.py | 23 ++++++++++ petab/models/sbml_model.py | 42 +++++++++++++++--- petab/observables.py | 26 ++++++----- petab/parameters.py | 90 ++++++++++++++++++++------------------ tests/test_lint.py | 18 ++++---- tests/test_observables.py | 3 +- 8 files changed, 178 insertions(+), 114 deletions(-) diff --git a/petab/lint.py b/petab/lint.py index e13f540b..0a1b5177 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -7,13 +7,13 @@ from typing import Optional, Iterable, Any from collections import Counter -import libsbml import numpy as np import pandas as pd import sympy as sp import petab -from . import (core, parameters, sbml, measurements) +from . import (core, parameters, measurements) +from .models import Model from .C import * # noqa: F403 logger = logging.getLogger(__name__) @@ -85,12 +85,12 @@ def assert_no_leading_trailing_whitespace( def check_condition_df( - df: pd.DataFrame, sbml_model: Optional[libsbml.Model] = None) -> None: + df: pd.DataFrame, model: Optional[Model] = None) -> None: """Run sanity checks on PEtab condition table Arguments: df: PEtab condition DataFrame - sbml_model: SBML Model for additional checking of parameter IDs + model: Model for additional checking of parameter IDs Raises: AssertionError: in case of problems @@ -117,16 +117,14 @@ def check_condition_df( assert_no_leading_trailing_whitespace( df[column_name].values, column_name) - if sbml_model is not None: + if model is not None: + allowed_cols = set(model.get_valid_ids_for_condition_table()) for column_name in df.columns: if column_name != CONDITION_NAME \ - and sbml_model.getParameter(column_name) is None \ - and sbml_model.getSpecies(column_name) is None \ - and sbml_model.getCompartment(column_name) is None: + and column_name not in allowed_cols: raise AssertionError( "Condition table contains column for unknown entity '" - f"{column_name}'. Column names must match parameter, " - "species or compartment IDs specified in the SBML model.") + f"{column_name}'.") def check_measurement_df(df: pd.DataFrame, @@ -189,7 +187,7 @@ def check_measurement_df(df: pd.DataFrame, def check_parameter_df( df: pd.DataFrame, - sbml_model: Optional[libsbml.Model] = None, + model: Optional[Model] = None, observable_df: Optional[pd.DataFrame] = None, measurement_df: Optional[pd.DataFrame] = None, condition_df: Optional[pd.DataFrame] = None) -> None: @@ -197,7 +195,7 @@ def check_parameter_df( Arguments: df: PEtab condition DataFrame - sbml_model: SBML Model for additional checking of parameter IDs + model: Model for additional checking of parameter IDs observable_df: PEtab observable table for additional checks measurement_df: PEtab measurement table for additional checks condition_df: PEtab condition table for additional checks @@ -247,10 +245,10 @@ def check_parameter_df( check_parameter_bounds(df) assert_parameter_prior_type_is_valid(df) - if sbml_model and measurement_df is not None \ + if model and measurement_df is not None \ and condition_df is not None: assert_all_parameters_present_in_parameter_df( - df, sbml_model, observable_df, measurement_df, condition_df) + df, model, observable_df, measurement_df, condition_df) def check_observable_df(observable_df: pd.DataFrame) -> None: @@ -306,7 +304,7 @@ def check_observable_df(observable_df: pd.DataFrame) -> None: def assert_all_parameters_present_in_parameter_df( parameter_df: pd.DataFrame, - sbml_model: libsbml.Model, + model: Model, observable_df: pd.DataFrame, measurement_df: pd.DataFrame, condition_df: pd.DataFrame) -> None: @@ -315,7 +313,7 @@ def assert_all_parameters_present_in_parameter_df( Arguments: parameter_df: PEtab parameter DataFrame - sbml_model: PEtab SBML Model + model: model observable_df: PEtab observable table measurement_df: PEtab measurement table condition_df: PEtab condition table @@ -325,11 +323,11 @@ def assert_all_parameters_present_in_parameter_df( """ required = parameters.get_required_parameters_for_parameter_table( - sbml_model=sbml_model, condition_df=condition_df, + model=model, condition_df=condition_df, observable_df=observable_df, measurement_df=measurement_df) allowed = parameters.get_valid_parameters_for_parameter_table( - sbml_model=sbml_model, condition_df=condition_df, + model=model, condition_df=condition_df, observable_df=observable_df, measurement_df=measurement_df) actual = set(parameter_df.index) @@ -764,13 +762,11 @@ def lint_problem(problem: 'petab.Problem') -> bool: errors_occurred = False # Run checks on individual files - if problem.sbml_model is not None: - logger.info("Checking SBML model...") - errors_occurred |= not sbml.is_sbml_consistent( - problem.sbml_model.getSBMLDocument()) - sbml.log_sbml_errors(problem.sbml_model.getSBMLDocument()) + if problem.model is not None: + logger.info("Checking model...") + errors_occurred |= problem.model.validate() else: - logger.warning("SBML model not available. Skipping.") + logger.warning("Model not available. Skipping.") if problem.measurement_df is not None: logger.info("Checking measurement table...") @@ -790,7 +786,7 @@ def lint_problem(problem: 'petab.Problem') -> bool: if problem.condition_df is not None: logger.info("Checking condition table...") try: - check_condition_df(problem.condition_df, problem.sbml_model) + check_condition_df(problem.condition_df, problem.model) except AssertionError as e: logger.error(e) errors_occurred = True @@ -804,9 +800,9 @@ def lint_problem(problem: 'petab.Problem') -> bool: except AssertionError as e: logger.error(e) errors_occurred = True - if problem.sbml_model is not None: + if problem.model is not None: for obs_id in problem.observable_df.index: - if problem.sbml_model.getElementBySId(obs_id): + if problem.model.has_entity_with_id(obs_id): logger.error(f"Observable ID {obs_id} shadows model " "entity.") errors_occurred = True @@ -816,7 +812,7 @@ def lint_problem(problem: 'petab.Problem') -> bool: if problem.parameter_df is not None: logger.info("Checking parameter table...") try: - check_parameter_df(problem.parameter_df, problem.sbml_model, + check_parameter_df(problem.parameter_df, problem.model, problem.observable_df, problem.measurement_df, problem.condition_df) except AssertionError as e: @@ -825,11 +821,11 @@ def lint_problem(problem: 'petab.Problem') -> bool: else: logger.warning("Parameter table not available. Skipping.") - if problem.sbml_model is not None and problem.condition_df is not None \ + if problem.model is not None and problem.condition_df is not None \ and problem.parameter_df is not None: try: assert_model_parameters_in_condition_or_parameter_table( - problem.sbml_model, + problem.model, problem.condition_df, problem.parameter_df ) @@ -840,7 +836,7 @@ def lint_problem(problem: 'petab.Problem') -> bool: if errors_occurred: logger.error('Not OK') elif problem.measurement_df is None or problem.condition_df is None \ - or problem.sbml_model is None or problem.parameter_df is None \ + or problem.model is None or problem.parameter_df is None \ or problem.observable_df is None: logger.warning('Not all files of the PEtab problem definition could ' 'be checked.') @@ -851,41 +847,42 @@ def lint_problem(problem: 'petab.Problem') -> bool: def assert_model_parameters_in_condition_or_parameter_table( - sbml_model: libsbml.Model, + model: Model, condition_df: pd.DataFrame, parameter_df: pd.DataFrame) -> None: - """Model parameters that are targets of AssignmentRule must not be present - in parameter table or in condition table columns. Other parameters must - only be present in either in parameter table or condition table columns. - Check that. + """Model parameters that are rule targets must not be present in the + parameter table. Other parameters must only be present in either in + parameter table or condition table columns. Check that. Arguments: parameter_df: PEtab parameter DataFrame - sbml_model: PEtab SBML Model + model: PEtab model condition_df: PEtab condition table Raises: AssertionError: in case of problems """ - for parameter in sbml_model.getListOfParameters(): - parameter_id = parameter.getId() + allowed_in_condition_cols = set(model.get_valid_ids_for_condition_table()) + allowed_in_parameter_table = \ + set(model.get_valid_parameters_for_parameter_table()) + for parameter_id in model.get_parameter_ids(): if parameter_id.startswith('observableParameter'): continue if parameter_id.startswith('noiseParameter'): continue - is_assignee = \ - sbml_model.getAssignmentRuleByVariable(parameter_id) is not None in_parameter_df = parameter_id in parameter_df.index in_condition_df = parameter_id in condition_df.columns - if is_assignee and (in_parameter_df or in_condition_df): - raise AssertionError(f"Model parameter '{parameter_id}' is target " - "of AssignmentRule, and thus, must not be " - "present in condition table or in parameter " - "table.") + if in_condition_df and parameter_id not in allowed_in_condition_cols: + raise AssertionError(f"Model parameter '{parameter_id}' is not " + "allowed to occur in condition table " + "columns.") + if in_parameter_df and parameter_id not in allowed_in_parameter_table: + raise AssertionError(f"Model parameter '{parameter_id}' is not " + "allowed to occur in the parameter table.") if in_parameter_df and in_condition_df: raise AssertionError(f"Model parameter '{parameter_id}' present " diff --git a/petab/models/__init__.py b/petab/models/__init__.py index e69de29b..d8afd107 100644 --- a/petab/models/__init__.py +++ b/petab/models/__init__.py @@ -0,0 +1 @@ +from .model import Model # noqa F401 diff --git a/petab/models/model.py b/petab/models/model.py index ab3c33aa..232863f5 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -10,6 +10,9 @@ class Model: def __init__(self): ... + # TODO more coherent method names / arguments + # TODO doc + # TODO remove unused @staticmethod @abc.abstractmethod @@ -20,6 +23,10 @@ def from_file(filepath_or_buffer: Any) -> Model: def get_parameter_ids(self) -> Iterable[str]: ... + @abc.abstractmethod + def get_parameter_value(self, id_: str) -> float: + ... + @abc.abstractmethod def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: ... @@ -40,6 +47,22 @@ def has_entity_with_id(self, entity_id) -> bool: def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: ... + @abc.abstractmethod + def get_valid_ids_for_condition_table(self) -> Iterable[str]: + ... + + @abc.abstractmethod + def symbol_allowed_in_observable_formula(self, id_: str) -> bool: + ... + + @abc.abstractmethod + def validate(self) -> bool: + """Validate model + + :returns: `True` if errors occurred, `False` otherwise. + """ + ... + def model_factory(filepath_or_buffer: Any, model_language: str) -> Model: """Create a PEtab model instance from the given model""" diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index cd5a19d8..6430b46f 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -1,9 +1,13 @@ """Functions for handling SBML models""" -from .model import Model -from ..sbml import get_sbml_model, load_sbml_from_string +import itertools +from typing import Iterable, Optional, Tuple + import libsbml -from typing import Optional, Iterable, Tuple + +from .model import Model +from ..sbml import (get_sbml_model, is_sbml_consistent, load_sbml_from_string, + log_sbml_errors) class SbmlModel(Model): @@ -62,6 +66,12 @@ def get_parameter_ids(self) -> Iterable[str]: if self.sbml_model.getAssignmentRuleByVariable(p.getId()) is None ) + def get_parameter_value(self, id_: str) -> float: + parameter = self.sbml_model.getParameter(id_) + if not parameter: + raise ValueError(f"Parameter {id_} does not exist.") + return parameter.getValue() + def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: return ( (p.getId(), p.getValue()) @@ -79,5 +89,27 @@ def has_entity_with_id(self, entity_id) -> bool: return self.sbml_model.getElementBySId(entity_id) is not None def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: - # TODO - pass + # exclude rule targets + disallowed_set = { + ar.getVariable() for ar in self.sbml_model.getListOfRules() + } + + return (p.getId() for p in self.sbml_model.getListOfParameters() + if p.getId() not in disallowed_set) + + def get_valid_ids_for_condition_table(self) -> Iterable[str]: + return ( + x.getId() for x in itertools.chain( + self.sbml_model.getListOfParameters(), + self.sbml_model.getListOfSpecies(), + self.sbml_model.getListOfCompartments() + ) + ) + + def symbol_allowed_in_observable_formula(self, id_: str) -> bool: + return self.sbml_model.getElementBySId(id_) or id_ == 'time' + + def validate(self) -> bool: + valid = is_sbml_consistent(self.sbml_model.getSBMLDocument()) + log_sbml_errors(self.sbml_model.getSBMLDocument()) + return not valid diff --git a/petab/observables.py b/petab/observables.py index 67fa1e4d..05fa142f 100644 --- a/petab/observables.py +++ b/petab/observables.py @@ -5,24 +5,26 @@ from pathlib import Path from typing import List, Union -import libsbml import pandas as pd import sympy as sp from . import core, lint from .C import * # noqa: F403 +from .models import Model -__all__ = ['create_observable_df', - 'get_formula_placeholders', - 'get_observable_df', - 'get_output_parameters', - 'get_placeholders', - 'write_observable_df'] +__all__ = [ + 'create_observable_df', + 'get_formula_placeholders', + 'get_observable_df', + 'get_output_parameters', + 'get_placeholders', + 'write_observable_df' +] def get_observable_df( observable_file: Union[str, pd.DataFrame, Path, None] -) -> pd.DataFrame: +) -> Union[pd.DataFrame, None]: """ Read the provided observable file into a ``pandas.Dataframe``. @@ -67,18 +69,18 @@ def write_observable_df(df: pd.DataFrame, filename: Union[str, Path]) -> None: def get_output_parameters( observable_df: pd.DataFrame, - sbml_model: libsbml.Model, + model: Model, observables: bool = True, noise: bool = True, ) -> List[str]: """Get output parameters Returns IDs of parameters used in observable and noise formulas that are - not defined in the SBML model. + not defined in the model. Arguments: observable_df: PEtab observable table - sbml_model: SBML model + model: The underlying model observables: Include parameters from observableFormulas noise: Include parameters from noiseFormulas @@ -97,7 +99,7 @@ def get_output_parameters( key=lambda symbol: symbol.name) for free_sym in free_syms: sym = str(free_sym) - if sbml_model.getElementBySId(sym) is None and sym != 'time': + if not model.symbol_allowed_in_observable_formula(sym): output_parameters[sym] = None return list(output_parameters.keys()) diff --git a/petab/parameters.py b/petab/parameters.py index 84bbdbef..1930cec7 100644 --- a/petab/parameters.py +++ b/petab/parameters.py @@ -1,9 +1,10 @@ """Functions operating on the PEtab parameter table""" import numbers +import warnings from collections import OrderedDict from pathlib import Path -from typing import Dict, Iterable, List, Set, Tuple, Union +from typing import Dict, Iterable, List, Set, Tuple, Union, Optional import libsbml import numpy as np @@ -11,14 +12,13 @@ from . import conditions, core, lint, measurements, observables from .C import * # noqa: F403 +from .models import Model __all__ = ['create_parameter_df', 'get_optimization_parameter_scaling', 'get_optimization_parameters', 'get_parameter_df', 'get_priors_from_df', - 'get_required_parameters_for_parameter_table', - 'get_valid_parameters_for_parameter_table', 'map_scale', 'map_unscale', 'normalize_parameter_df', @@ -124,14 +124,17 @@ def get_optimization_parameter_scaling( return dict(zip(estimated_df.index, estimated_df[PARAMETER_SCALE])) -def create_parameter_df(sbml_model: libsbml.Model, - condition_df: pd.DataFrame, - observable_df: pd.DataFrame, - measurement_df: pd.DataFrame, - include_optional: bool = False, - parameter_scale: str = LOG10, - lower_bound: Iterable = None, - upper_bound: Iterable = None) -> pd.DataFrame: +def create_parameter_df( + sbml_model: Optional[libsbml.Model] = None, + condition_df: Optional[pd.DataFrame] = None, + observable_df: Optional[pd.DataFrame] = None, + measurement_df: Optional[pd.DataFrame] = None, + model: Optional[Model] = None, + include_optional: bool = False, + parameter_scale: str = LOG10, + lower_bound: Iterable = None, + upper_bound: Iterable = None +) -> pd.DataFrame: """Create a new PEtab parameter table All table entries can be provided as string or list-like with length @@ -146,7 +149,7 @@ def create_parameter_df(sbml_model: libsbml.Model, required to be present in the parameter table. If set to True, this returns all parameters that are allowed to be present in the parameter table (i.e. also including parameters specified in the - SBML model). + model). parameter_scale: parameter scaling lower_bound: lower bound for parameter value upper_bound: upper bound for parameter value @@ -154,14 +157,23 @@ def create_parameter_df(sbml_model: libsbml.Model, Returns: The created parameter DataFrame """ - + if sbml_model: + warnings.warn("Passing a model via the `sbml_model` argument is " + "deprecated, use `model=petab.models.sbml_model." + "SbmlModel(...)` instead.", DeprecationWarning, + stacklevel=2) + from petab.models.sbml_model import SbmlModel + if model: + raise ValueError("Arguments `model` and `sbml_model` are " + "mutually exclusive.") + model = SbmlModel(sbml_model=sbml_model) if include_optional: parameter_ids = list(get_valid_parameters_for_parameter_table( - sbml_model=sbml_model, condition_df=condition_df, + model=model, condition_df=condition_df, observable_df=observable_df, measurement_df=measurement_df)) else: parameter_ids = list(get_required_parameters_for_parameter_table( - sbml_model=sbml_model, condition_df=condition_df, + model=model, condition_df=condition_df, observable_df=observable_df, measurement_df=measurement_df)) df = pd.DataFrame( @@ -180,12 +192,11 @@ def create_parameter_df(sbml_model: libsbml.Model, }) df.set_index([PARAMETER_ID], inplace=True) - # For SBML model parameters, set nominal values as defined in the model + # For model parameters, set nominal values as defined in the model for parameter_id in df.index: try: - parameter = sbml_model.getParameter(parameter_id) - if parameter: - df.loc[parameter_id, NOMINAL_VALUE] = parameter.getValue() + df.loc[parameter_id, NOMINAL_VALUE] = \ + model.get_parameter_value(parameter_id) except ValueError: # parameter was introduced as condition-specific override and # is potentially not present in the model @@ -194,7 +205,7 @@ def create_parameter_df(sbml_model: libsbml.Model, def get_required_parameters_for_parameter_table( - sbml_model: libsbml.Model, + model: Model, condition_df: pd.DataFrame, observable_df: pd.DataFrame, measurement_df: pd.DataFrame) -> Set[str]: @@ -202,7 +213,7 @@ def get_required_parameters_for_parameter_table( Get set of parameters which need to go into the parameter table Arguments: - sbml_model: PEtab SBML model + model: PEtab model condition_df: PEtab condition table observable_df: PEtab observable table measurement_df: PEtab measurement table @@ -211,7 +222,7 @@ def get_required_parameters_for_parameter_table( Set of parameter IDs which PEtab requires to be present in the parameter table. That is all {observable,noise}Parameters from the measurement table as well as all parametric condition table overrides - that are not defined in the SBML model. + that are not defined in the model. """ # use ordered dict as proxy for ordered set @@ -231,27 +242,28 @@ def append_overrides(overrides): row.get(NOISE_PARAMETERS, None))) # Add output parameters except for placeholders + model_parameter_ids = set(model.get_parameter_ids()) for kwargs in [dict(observables=True, noise=False), dict(observables=False, noise=True)]: output_parameters = observables.get_output_parameters( - observable_df, sbml_model, **kwargs) + observable_df, model, **kwargs) placeholders = observables.get_placeholders( observable_df, **kwargs) for p in output_parameters: - if p not in placeholders and sbml_model.getParameter(p) is None: + if p not in placeholders and p not in model_parameter_ids: parameter_ids[p] = None # Add condition table parametric overrides unless already defined in the - # SBML model + # model for p in conditions.get_parametric_overrides(condition_df): - if sbml_model.getParameter(p) is None: + if p not in model_parameter_ids: parameter_ids[p] = None return parameter_ids.keys() def get_valid_parameters_for_parameter_table( - sbml_model: libsbml.Model, + model: Model, condition_df: pd.DataFrame, observable_df: pd.DataFrame, measurement_df: pd.DataFrame) -> Set[str]: @@ -259,7 +271,7 @@ def get_valid_parameters_for_parameter_table( Get set of parameters which may be present inside the parameter table Arguments: - sbml_model: PEtab SBML model + model: PEtab model condition_df: PEtab condition table observable_df: PEtab observable table measurement_df: PEtab measurement table @@ -268,38 +280,32 @@ def get_valid_parameters_for_parameter_table( Set of parameter IDs which PEtab allows to be present in the parameter table. """ - - # - grab all model parameters + # - grab all allowed model parameters # - grab all output parameters defined in {observable,noise}Formula # - grab all parameters from measurement table # - grab all parametric overrides from condition table # - remove parameters for which condition table columns exist - # - remove observables assigment targets - # - remove sigma assignment targets # - remove placeholder parameters # (only partial overrides are not supported) placeholders = set(observables.get_placeholders(observable_df)) - # exclude rule targets - assignment_targets = {ar.getVariable() - for ar in sbml_model.getListOfRules()} - # must not go into parameter table blackset = set() # collect assignment targets blackset |= placeholders - blackset |= assignment_targets blackset |= set(condition_df.columns.values) - {CONDITION_NAME} - # use ordered dict as proxy for ordered set + # don't use sets here, to have deterministic ordering, + # e.g. for creating parameter tables parameter_ids = OrderedDict.fromkeys( - p.getId() for p in sbml_model.getListOfParameters() - if p.getId() not in blackset) + p for p in model.get_valid_parameters_for_parameter_table() + if p not in blackset + ) # add output parameters from observables table output_parameters = observables.get_output_parameters( - observable_df, sbml_model) + observable_df=observable_df, model=model) for p in output_parameters: if p not in blackset: parameter_ids[p] = None @@ -353,7 +359,7 @@ def get_priors_from_df(parameter_df: pd.DataFrame, lb, ub = map_scale([row[LOWER_BOUND], row[UPPER_BOUND]], [row[PARAMETER_SCALE]] * 2) pars_str = f'{lb};{ub}' - prior_pars = tuple([float(entry) for entry in pars_str.split(';')]) + prior_pars = tuple(float(entry) for entry in pars_str.split(';')) # add parameter scale and bounds, as this may be needed par_scale = row[PARAMETER_SCALE] diff --git a/tests/test_lint.py b/tests/test_lint.py index 85ea6626..f1935add 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -211,11 +211,12 @@ def test_assert_no_leading_trailing_whitespace(): def test_assert_model_parameters_in_condition_or_parameter_table(): import simplesbml + from petab.models.sbml_model import SbmlModel ss_model = simplesbml.SbmlModel() ss_model.addParameter('parameter1', 0.0) ss_model.addParameter('noiseParameter1_', 0.0) ss_model.addParameter('observableParameter1_', 0.0) - sbml_model = ss_model.model + sbml_model = SbmlModel(sbml_model=ss_model.model) lint.assert_model_parameters_in_condition_or_parameter_table( sbml_model, pd.DataFrame(columns=['parameter1']), pd.DataFrame() @@ -405,8 +406,9 @@ def test_assert_measurement_conditions_present_in_condition_table(): def test_check_condition_df(): """Check that we correctly detect errors in condition table""" import simplesbml + from petab.models.sbml_model import SbmlModel ss_model = simplesbml.SbmlModel() - + model = SbmlModel(sbml_model=ss_model.model) condition_df = pd.DataFrame(data={ CONDITION_ID: ['condition1'], 'p1': [nan], @@ -415,29 +417,29 @@ def test_check_condition_df(): # parameter missing in model with pytest.raises(AssertionError): - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) # fix: ss_model.addParameter('p1', 1.0) - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) # species missing in model condition_df['s1'] = [3.0] with pytest.raises(AssertionError): - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) # fix: ss_model.addSpecies("[s1]", 1.0) - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) # compartment missing in model condition_df['c2'] = [4.0] with pytest.raises(AssertionError): - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) # fix: ss_model.addCompartment(comp_id='c2', vol=1.0) - lint.check_condition_df(condition_df, ss_model.model) + lint.check_condition_df(condition_df, model) def test_check_ids(): diff --git a/tests/test_observables.py b/tests/test_observables.py index d18c20a4..4492e8a1 100644 --- a/tests/test_observables.py +++ b/tests/test_observables.py @@ -65,6 +65,7 @@ def test_get_output_parameters(): """Test measurements.get_output_parameters.""" # sbml model import simplesbml + from petab.models.sbml_model import SbmlModel ss_model = simplesbml.SbmlModel() ss_model.addParameter('fixedParameter1', 1.0) ss_model.addParameter('observable_1', 1.0) @@ -78,7 +79,7 @@ def test_get_output_parameters(): }).set_index(OBSERVABLE_ID) output_parameters = petab.get_output_parameters( - observable_df, ss_model.model) + observable_df, SbmlModel(sbml_model=ss_model.model)) assert output_parameters == ['offset', 'scaling'] From d7986521ec980aa194a1fcb53995080f12015ed7 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 4 May 2022 12:45:08 +0200 Subject: [PATCH 07/28] doc --- petab/lint.py | 2 +- petab/models/model.py | 57 +++++++++++++++++++++++++++++++++++--- petab/models/sbml_model.py | 2 +- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/petab/lint.py b/petab/lint.py index 0a1b5177..b5417cb0 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -764,7 +764,7 @@ def lint_problem(problem: 'petab.Problem') -> bool: # Run checks on individual files if problem.model is not None: logger.info("Checking model...") - errors_occurred |= problem.model.validate() + errors_occurred |= not problem.model.is_valid() else: logger.warning("Model not available. Skipping.") diff --git a/petab/models/model.py b/petab/models/model.py index 232863f5..61b296e0 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -17,55 +17,104 @@ def __init__(self): @staticmethod @abc.abstractmethod def from_file(filepath_or_buffer: Any) -> Model: + """Load the model from the given path/URL + + :param filepath_or_buffer: URL or path of the model + :returns: A ``Model`` instance holding the given model + """ ... @abc.abstractmethod def get_parameter_ids(self) -> Iterable[str]: + """Get all parameter IDs from this model + + :returns: Iterator over model parameter IDs + """ ... @abc.abstractmethod def get_parameter_value(self, id_: str) -> float: + """Get a parameter value + + :param id_: ID of the parameter whose value is to be returned + :raises ValueError: If no parameter with the given ID exists + :returns: The value of the given parameter as specified in the model + """ ... @abc.abstractmethod def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: + # TODO yet unused ... @abc.abstractmethod def has_species_with_id(self, entity_id: str) -> bool: + # TODO yet unused ... @abc.abstractmethod def has_compartment_with_id(self, entity_id: str) -> bool: + # TODO yet unused ... @abc.abstractmethod def has_entity_with_id(self, entity_id) -> bool: + """Check if there is a model entity with the given ID + + :param entity_id: ID to check for + :returns: ``True``, if there is an entity with the given ID, + ``False`` otherwise + """ ... @abc.abstractmethod def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: + """Get IDs of all parameters that are allowed to occur in the PEtab + parameters table + + :returns: Iterator over parameter IDs + """ ... @abc.abstractmethod def get_valid_ids_for_condition_table(self) -> Iterable[str]: + """Get IDs of all model entities that are allowed to occur as columns + in the PEtab conditions table. + + :returns: Iterator over model entity IDs + """ ... @abc.abstractmethod def symbol_allowed_in_observable_formula(self, id_: str) -> bool: + """Check if the given ID is allowed to be used in observable formulas + + # TODO currently also used for noise + + :returns: ``True``, if allowed, ``False`` otherwise + """ + ... @abc.abstractmethod - def validate(self) -> bool: - """Validate model + def is_valid(self) -> bool: + """Validate this model - :returns: `True` if errors occurred, `False` otherwise. + # TODO optional printing? + :returns: `True` if the model is valid, `False` if there are errors in + this model """ ... def model_factory(filepath_or_buffer: Any, model_language: str) -> Model: - """Create a PEtab model instance from the given model""" + """Create a PEtab model instance from the given model + + :param filepath_or_buffer: Path/URL of the model + :param model_language: PEtab model language ID for the given model + :returns: A :py:class:`Model` instance representing the given model + """ + if model_language == "sbml": from .sbml_model import SbmlModel return SbmlModel.from_file(filepath_or_buffer) diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 6430b46f..8fb8aadf 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -109,7 +109,7 @@ def get_valid_ids_for_condition_table(self) -> Iterable[str]: def symbol_allowed_in_observable_formula(self, id_: str) -> bool: return self.sbml_model.getElementBySId(id_) or id_ == 'time' - def validate(self) -> bool: + def is_valid(self) -> bool: valid = is_sbml_consistent(self.sbml_model.getSBMLDocument()) log_sbml_errors(self.sbml_model.getSBMLDocument()) return not valid From 9a34c746942f1d1cb8bd436c42a89360121aeef8 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 4 May 2022 14:12:39 +0200 Subject: [PATCH 08/28] .. --- petab/models/sbml_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 8fb8aadf..f6a496df 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -112,4 +112,4 @@ def symbol_allowed_in_observable_formula(self, id_: str) -> bool: def is_valid(self) -> bool: valid = is_sbml_consistent(self.sbml_model.getSBMLDocument()) log_sbml_errors(self.sbml_model.getSBMLDocument()) - return not valid + return valid From e0b6e74a010962e4a55dc2f1f4705201b30b7e25 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 5 May 2022 10:37:31 +0200 Subject: [PATCH 09/28] Deprecate petab.Problem.from_files --- petab/problem.py | 74 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/petab/problem.py b/petab/problem.py index 23579f0d..0ca89c70 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -13,7 +13,7 @@ from . import (conditions, core, format_version, measurements, observables, parameter_mapping, parameters, sampling, sbml, yaml) from .C import * # noqa: F403 -from .models.model import Model +from .models.model import Model, model_factory from .models.sbml_model import SbmlModel if TYPE_CHECKING: @@ -27,7 +27,7 @@ class Problem: """ PEtab parameter estimation problem as defined by - - SBML model + - model - condition table - measurement table - parameter table @@ -41,9 +41,10 @@ class Problem: parameter_df: PEtab parameter table observable_df: PEtab observable table visualization_df: PEtab visualization table - sbml_reader: Stored to keep object alive. - sbml_document: Stored to keep object alive. - sbml_model: PEtab SBML model + model: The underlying model + sbml_reader: Stored to keep object alive (deprecated). + sbml_document: Stored to keep object alive (deprecated). + sbml_model: PEtab SBML model (deprecated) """ def __init__(self, @@ -109,7 +110,7 @@ def from_files( visualization_files: Union[str, Path, Iterable[Union[str, Path]]] = None, observable_files: Union[str, Path, - Iterable[Union[str, Path]]] = None + Iterable[Union[str, Path]]] = None, ) -> 'Problem': """ Factory method to load model and tables from files. @@ -122,6 +123,9 @@ def from_files( visualization_files: PEtab visualization tables observable_files: PEtab observables tables """ + warn("petab.Problem.from_files is deprecated and will be removed in a " + "future version. Use `petab.Problem.from_yaml instead.", + DeprecationWarning, stacklevel=2) sbml_model = sbml_document = sbml_reader = None condition_df = measurement_df = parameter_df = visualization_df = None @@ -214,25 +218,53 @@ def from_yaml(yaml_config: Union[Dict, Path, str]) -> 'Problem': yaml.assert_single_condition_and_sbml_file(problem0) if isinstance(yaml_config[PARAMETER_FILE], list): - parameter_file = [ - get_path(f) for f in yaml_config[PARAMETER_FILE] + parameter_df = [ + parameters.get_parameter_df(get_path(f)) + for f in yaml_config[PARAMETER_FILE] ] else: - parameter_file = get_path(yaml_config[PARAMETER_FILE]) \ + parameter_df = parameters.get_parameter_df( + get_path(yaml_config[PARAMETER_FILE])) \ if yaml_config[PARAMETER_FILE] else None - return Problem.from_files( - sbml_file=get_path(problem0[SBML_FILES][0]) - if problem0[SBML_FILES] else None, - measurement_file=[get_path(f) - for f in problem0[MEASUREMENT_FILES]], - condition_file=get_path(problem0[CONDITION_FILES][0]), - parameter_file=parameter_file, - visualization_files=[ - get_path(f) for f in problem0.get(VISUALIZATION_FILES, [])], - observable_files=[ - get_path(f) for f in problem0.get(OBSERVABLE_FILES, [])] - ) + model = model_factory(get_path(problem0[SBML_FILES][0]), 'sbml') \ + if problem0[SBML_FILES] else None + + if problem0[MEASUREMENT_FILES]: + measurement_files = [ + get_path(f) for f in problem0[MEASUREMENT_FILES] + ] + # If there are multiple tables, we will merge them + measurement_df = core.concat_tables( + measurement_files, measurements.get_measurement_df) \ + if measurement_files else None + else: + measurement_df = None + + condition_df = conditions.get_condition_df( + get_path(problem0[CONDITION_FILES][0])) \ + if problem0[CONDITION_FILES][0] else None + + visualization_files = [ + get_path(f) for f in problem0.get(VISUALIZATION_FILES, [])] + # If there are multiple tables, we will merge them + visualization_df = core.concat_tables( + visualization_files, core.get_visualization_df) \ + if visualization_files else None + + observable_files = [ + get_path(f) for f in problem0.get(OBSERVABLE_FILES, [])] + # If there are multiple tables, we will merge them + observable_df = core.concat_tables( + observable_files, observables.get_observable_df) \ + if observable_files else None + + return Problem(condition_df=condition_df, + measurement_df=measurement_df, + parameter_df=parameter_df, + observable_df=observable_df, + model=model, + visualization_df=visualization_df) @staticmethod def from_combine(filename: Union[Path, str]) -> 'Problem': From 2924c2c14d862cf21a0956a3c82f99ed18776831 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 5 May 2022 10:50:36 +0200 Subject: [PATCH 10/28] Update petab.Problem to use Model --- petab/problem.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/petab/problem.py b/petab/problem.py index 0ca89c70..acd0a73e 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -337,7 +337,10 @@ def to_files_generic( if getattr(self, f'{table_name}_df') is not None: filenames[f'{table_name}_file'] = f'{table_name}s.tsv' - if self.sbml_document is not None: + if self.model: + if not isinstance(self.model, SbmlModel): + raise NotImplementedError("Saving non-SBML models is " + "currently not supported.") filenames['sbml_file'] = 'model.xml' filenames['yaml_file'] = 'problem.yaml' @@ -475,6 +478,11 @@ def get_optimization_parameter_scales(self): def get_model_parameters(self): """See :py:func:`petab.sbml.get_model_parameters`""" + warn("petab.Problem.get_model_parameters is deprecated and will be " + "removed in a future version. Use " + "`petab.Problem.model.get_parameter_ids` instead.", + DeprecationWarning, stacklevel=2) + return sbml.get_model_parameters(self.sbml_model) def get_observable_ids(self): @@ -713,10 +721,10 @@ def create_parameter_df(self, *args, **kwargs): See :py:func:`create_parameter_df`. """ return parameters.create_parameter_df( - self.sbml_model, - self.condition_df, - self.observable_df, - self.measurement_df, + model=self.model, + condition_df=self.condition_df, + observable_df=self.observable_df, + measurement_df=self.measurement_df, *args, **kwargs) def sample_parameter_startpoints(self, n_starts: int = 100): From 507c1962be7adaa29b7a81be55ddf4389182fa3e Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 5 May 2022 11:05:47 +0200 Subject: [PATCH 11/28] Update to test_petab --- petab/problem.py | 46 +++++++++++++++++----------------------- tests/test_petab.py | 51 ++++++++++++++++++++++++++++++--------------- 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/petab/problem.py b/petab/problem.py index acd0a73e..3040635a 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -127,42 +127,34 @@ def from_files( "future version. Use `petab.Problem.from_yaml instead.", DeprecationWarning, stacklevel=2) - sbml_model = sbml_document = sbml_reader = None - condition_df = measurement_df = parameter_df = visualization_df = None - observable_df = None + model = model_factory(sbml_file, 'sbml') if sbml_file else None - if condition_file: - condition_df = conditions.get_condition_df(condition_file) - - if measurement_file: - # If there are multiple tables, we will merge them - measurement_df = core.concat_tables( - measurement_file, measurements.get_measurement_df) + condition_df = conditions.get_condition_df(condition_file) \ + if condition_file else None - if parameter_file: - parameter_df = parameters.get_parameter_df(parameter_file) + # If there are multiple tables, we will merge them + measurement_df = core.concat_tables( + measurement_file, measurements.get_measurement_df) \ + if measurement_file else None - if sbml_file: - sbml_reader, sbml_document, sbml_model = \ - sbml.get_sbml_model(sbml_file) + parameter_df = parameters.get_parameter_df(parameter_file) \ + if parameter_file else None - if visualization_files: - # If there are multiple tables, we will merge them - visualization_df = core.concat_tables( - visualization_files, core.get_visualization_df) + # If there are multiple tables, we will merge them + visualization_df = core.concat_tables( + visualization_files, core.get_visualization_df) \ + if visualization_files else None - if observable_files: - # If there are multiple tables, we will merge them - observable_df = core.concat_tables( - observable_files, observables.get_observable_df) + # If there are multiple tables, we will merge them + observable_df = core.concat_tables( + observable_files, observables.get_observable_df) \ + if observable_files else None - return Problem(condition_df=condition_df, + return Problem(model=model, + condition_df=condition_df, measurement_df=measurement_df, parameter_df=parameter_df, observable_df=observable_df, - sbml_model=sbml_model, - sbml_document=sbml_document, - sbml_reader=sbml_reader, visualization_df=visualization_df) @staticmethod diff --git a/tests/test_petab.py b/tests/test_petab.py index 62e74e82..94a1bce3 100644 --- a/tests/test_petab.py +++ b/tests/test_petab.py @@ -1,15 +1,18 @@ import copy import pickle import tempfile +import warnings from math import nan from pathlib import Path import libsbml import numpy as np import pandas as pd -import petab import pytest + +import petab from petab.C import * +from petab.models.sbml_model import SbmlModel @pytest.fixture @@ -201,6 +204,7 @@ def test_create_parameter_df( ss_model.addSpecies('[x1]', 1.0) ss_model.addParameter('fixedParameter1', 2.0) ss_model.addParameter('p0', 3.0) + model = SbmlModel(sbml_model=ss_model.model) observable_df = pd.DataFrame(data={ OBSERVABLE_ID: ['obs1', 'obs2'], @@ -217,16 +221,28 @@ def test_create_parameter_df( NOISE_PARAMETERS: ['p3;p4', 'p5'] }) - parameter_df = petab.create_parameter_df( - ss_model.model, - condition_df_2_conditions, - observable_df, - measurement_df) - # first model parameters, then row by row noise and sigma overrides expected = ['p3', 'p4', 'p1', 'p2', 'p5'] - actual = parameter_df.index.values.tolist() - assert actual == expected + + # Test old API with passing libsbml.Model directly + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + parameter_df = petab.create_parameter_df( + ss_model.model, + condition_df_2_conditions, + observable_df, + measurement_df) + assert len(w) == 1 + assert issubclass(w[-1].category, DeprecationWarning) + assert parameter_df.index.values.tolist() == expected + + parameter_df = petab.create_parameter_df( + model=model, + condition_df=condition_df_2_conditions, + observable_df=observable_df, + measurement_df=measurement_df + ) + assert parameter_df.index.values.tolist() == expected # test with condition parameter override: condition_df_2_conditions.loc['condition2', 'fixedParameter1'] \ @@ -234,10 +250,11 @@ def test_create_parameter_df( expected = ['p3', 'p4', 'p1', 'p2', 'p5', 'overrider'] parameter_df = petab.create_parameter_df( - ss_model.model, - condition_df_2_conditions, - observable_df, - measurement_df) + model=model, + condition_df=condition_df_2_conditions, + observable_df=observable_df, + measurement_df=measurement_df, + ) actual = parameter_df.index.values.tolist() assert actual == expected @@ -245,10 +262,10 @@ def test_create_parameter_df( expected = ['p0', 'p3', 'p4', 'p1', 'p2', 'p5', 'overrider'] parameter_df = petab.create_parameter_df( - ss_model.model, - condition_df_2_conditions, - observable_df, - measurement_df, + model=model, + condition_df=condition_df_2_conditions, + observable_df=observable_df, + measurement_df=measurement_df, include_optional=True) actual = parameter_df.index.values.tolist() assert actual == expected From 7d9dae8b4a8e885b4b7a0bce48c52b323c9f9383 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 5 May 2022 13:12:40 +0200 Subject: [PATCH 12/28] Add model type constants; add type code to class; add list of known types --- petab/models/__init__.py | 4 ++++ petab/models/model.py | 16 +++++++++++++--- petab/models/sbml_model.py | 4 ++++ petab/problem.py | 7 +++++-- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/petab/models/__init__.py b/petab/models/__init__.py index d8afd107..b4bc9601 100644 --- a/petab/models/__init__.py +++ b/petab/models/__init__.py @@ -1 +1,5 @@ +MODEL_TYPE_SBML = 'sbml' + +known_model_types = {MODEL_TYPE_SBML} + from .model import Model # noqa F401 diff --git a/petab/models/model.py b/petab/models/model.py index 61b296e0..cfc94ab4 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -3,6 +3,7 @@ import abc from typing import Any, Iterable, Tuple +from . import MODEL_TYPE_SBML, known_model_types class Model: @@ -24,6 +25,12 @@ def from_file(filepath_or_buffer: Any) -> Model: """ ... + @classmethod + @property + @abc.abstractmethod + def type_id(cls): + ... + @abc.abstractmethod def get_parameter_ids(self) -> Iterable[str]: """Get all parameter IDs from this model @@ -114,9 +121,12 @@ def model_factory(filepath_or_buffer: Any, model_language: str) -> Model: :param model_language: PEtab model language ID for the given model :returns: A :py:class:`Model` instance representing the given model """ - - if model_language == "sbml": + if model_language == MODEL_TYPE_SBML: from .sbml_model import SbmlModel return SbmlModel.from_file(filepath_or_buffer) - raise ValueError(f"Unsupported model format: {model_language}") + if model_language in known_model_types: + raise NotImplementedError( + f"Unsupported model format: {model_language}") + + raise ValueError(f"Unknown model format: {model_language}") diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index f6a496df..62aa887d 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -5,6 +5,7 @@ import libsbml +from . import MODEL_TYPE_SBML from .model import Model from ..sbml import (get_sbml_model, is_sbml_consistent, load_sbml_from_string, log_sbml_errors) @@ -12,6 +13,9 @@ class SbmlModel(Model): """PEtab wrapper for SBML models""" + + type_id = MODEL_TYPE_SBML + def __init__( self, sbml_model: libsbml.Model = None, diff --git a/petab/problem.py b/petab/problem.py index 3040635a..faed0cf2 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -13,6 +13,7 @@ from . import (conditions, core, format_version, measurements, observables, parameter_mapping, parameters, sampling, sbml, yaml) from .C import * # noqa: F403 +from .models import MODEL_TYPE_SBML from .models.model import Model, model_factory from .models.sbml_model import SbmlModel @@ -127,7 +128,8 @@ def from_files( "future version. Use `petab.Problem.from_yaml instead.", DeprecationWarning, stacklevel=2) - model = model_factory(sbml_file, 'sbml') if sbml_file else None + model = model_factory(sbml_file, MODEL_TYPE_SBML) \ + if sbml_file else None condition_df = conditions.get_condition_df(condition_file) \ if condition_file else None @@ -219,7 +221,8 @@ def from_yaml(yaml_config: Union[Dict, Path, str]) -> 'Problem': get_path(yaml_config[PARAMETER_FILE])) \ if yaml_config[PARAMETER_FILE] else None - model = model_factory(get_path(problem0[SBML_FILES][0]), 'sbml') \ + model = model_factory(get_path(problem0[SBML_FILES][0]), + MODEL_TYPE_SBML) \ if problem0[SBML_FILES] else None if problem0[MEASUREMENT_FILES]: From 3d4cda5e32550dd321662298733c822446140001 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 5 May 2022 13:22:57 +0200 Subject: [PATCH 13/28] Model.to_file --- petab/models/model.py | 10 ++++++++++ petab/models/sbml_model.py | 6 ++++++ petab/problem.py | 18 +++++++++++++----- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/petab/models/model.py b/petab/models/model.py index cfc94ab4..00270204 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -2,7 +2,9 @@ from __future__ import annotations import abc +from pathlib import Path from typing import Any, Iterable, Tuple + from . import MODEL_TYPE_SBML, known_model_types @@ -25,6 +27,14 @@ def from_file(filepath_or_buffer: Any) -> Model: """ ... + @abc.abstractmethod + def to_file(self, filename: [str, Path]): + """Save the model to the given file + + :param filename: Destination filename + """ + ... + @classmethod @property @abc.abstractmethod diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 62aa887d..bac5c675 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -1,6 +1,7 @@ """Functions for handling SBML models""" import itertools +from pathlib import Path from typing import Iterable, Optional, Tuple import libsbml @@ -64,6 +65,11 @@ def from_file(filepath_or_buffer): sbml_document=sbml_document, ) + def to_file(self, filename: [str, Path]): + from ..sbml import write_sbml + write_sbml(self.sbml_document or self.sbml_model.getSBMLDocument(), + filename) + def get_parameter_ids(self) -> Iterable[str]: return ( p.getId() for p in self.sbml_model.getListOfParameters() diff --git a/petab/problem.py b/petab/problem.py index faed0cf2..d9549a13 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -356,6 +356,7 @@ def to_files(self, yaml_file: Union[None, str, Path] = None, prefix_path: Union[None, str, Path] = None, relative_paths: bool = True, + model_file: Union[None, str, Path] = None, ) -> None: """ Write PEtab tables to files for this problem @@ -404,11 +405,18 @@ def add_prefix(path0: Union[None, str, Path]) -> str: yaml_file = add_prefix(yaml_file) if sbml_file: - if self.sbml_document is not None: - sbml.write_sbml(self.sbml_document, sbml_file) - else: - raise ValueError("Unable to save SBML model with no " - "sbml_doc set.") + warn("The `sbml_file` argument is deprecated and will be " + "removed in a future version. Use `model_file` instead.", + DeprecationWarning, stacklevel=2) + + if model_file: + raise ValueError("Must provide either `sbml_file` or " + "`model_file` argument, but not both.") + + model_file = sbml_file + + if model_file: + self.model.to_file(model_file) def error(name: str) -> ValueError: return ValueError(f"Unable to save non-existent {name} table") From d8d177494cce8c4dc0b49bde40ce007c64b0cb0f Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 19 May 2022 21:56:06 +0200 Subject: [PATCH 14/28] fixup, doc --- petab/parameters.py | 1 + petab/problem.py | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/petab/parameters.py b/petab/parameters.py index 1930cec7..af514f64 100644 --- a/petab/parameters.py +++ b/petab/parameters.py @@ -142,6 +142,7 @@ def create_parameter_df( Arguments: sbml_model: SBML Model + model: PEtab model condition_df: PEtab condition DataFrame observable_df: PEtab observable DataFrame measurement_df: PEtab measurement DataFrame diff --git a/petab/problem.py b/petab/problem.py index d9549a13..c950a6aa 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -369,6 +369,7 @@ def to_files(self, Arguments: sbml_file: SBML model destination + model_file: Model destination condition_file: Condition table destination measurement_file: Measurement table destination parameter_file: Parameter table destination @@ -388,6 +389,17 @@ def to_files(self, ValueError: If a destination was provided for a non-existing entity. """ + if sbml_file: + warn("The `sbml_file` argument is deprecated and will be " + "removed in a future version. Use `model_file` instead.", + DeprecationWarning, stacklevel=2) + + if model_file: + raise ValueError("Must provide either `sbml_file` or " + "`model_file` argument, but not both.") + + model_file = sbml_file + if prefix_path is not None: prefix_path = Path(prefix_path) @@ -396,7 +408,7 @@ def add_prefix(path0: Union[None, str, Path]) -> str: return path0 return str(prefix_path / path0) - sbml_file = add_prefix(sbml_file) + model_file = add_prefix(model_file) condition_file = add_prefix(condition_file) measurement_file = add_prefix(measurement_file) parameter_file = add_prefix(parameter_file) @@ -404,17 +416,6 @@ def add_prefix(path0: Union[None, str, Path]) -> str: visualization_file = add_prefix(visualization_file) yaml_file = add_prefix(yaml_file) - if sbml_file: - warn("The `sbml_file` argument is deprecated and will be " - "removed in a future version. Use `model_file` instead.", - DeprecationWarning, stacklevel=2) - - if model_file: - raise ValueError("Must provide either `sbml_file` or " - "`model_file` argument, but not both.") - - model_file = sbml_file - if model_file: self.model.to_file(model_file) From ee977a78453df9db98a0181a79cfc713191be8da Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 19 May 2022 22:22:22 +0200 Subject: [PATCH 15/28] cleanup assert_model_parameters_in_condition_or_parameter_table --- petab/lint.py | 50 +++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/petab/lint.py b/petab/lint.py index b5417cb0..660f045e 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -862,32 +862,36 @@ def assert_model_parameters_in_condition_or_parameter_table( Raises: AssertionError: in case of problems """ - allowed_in_condition_cols = set(model.get_valid_ids_for_condition_table()) allowed_in_parameter_table = \ set(model.get_valid_parameters_for_parameter_table()) - - for parameter_id in model.get_parameter_ids(): - if parameter_id.startswith('observableParameter'): - continue - if parameter_id.startswith('noiseParameter'): - continue - - in_parameter_df = parameter_id in parameter_df.index - in_condition_df = parameter_id in condition_df.columns - - if in_condition_df and parameter_id not in allowed_in_condition_cols: - raise AssertionError(f"Model parameter '{parameter_id}' is not " - "allowed to occur in condition table " - "columns.") - if in_parameter_df and parameter_id not in allowed_in_parameter_table: - raise AssertionError(f"Model parameter '{parameter_id}' is not " - "allowed to occur in the parameter table.") - - if in_parameter_df and in_condition_df: - raise AssertionError(f"Model parameter '{parameter_id}' present " - "in both condition table and parameter " - "table.") + entities_in_condition_table = set(condition_df.columns) - {CONDITION_NAME} + entities_in_parameter_table = set(parameter_df.index.values) + + disallowed_in_condition = { + x for x in (entities_in_condition_table - allowed_in_condition_cols) + # we only check model entities here, not output parameters + if model.has_entity_with_id(x) + } + if disallowed_in_condition: + raise AssertionError(f"{disallowed_in_condition} is not " + "allowed to occur in condition table " + "columns.") + + disallowed_in_parameters = { + x for x in (entities_in_parameter_table - allowed_in_parameter_table) + # we only check model entities here, not output parameters + if model.has_entity_with_id(x) + } + + if disallowed_in_parameters: + raise AssertionError(f"{disallowed_in_parameters} is not " + "allowed to occur in the parameters table.") + + in_both = entities_in_condition_table & entities_in_parameter_table + if in_both: + raise AssertionError(f"{in_both} are present in both condition table " + "and parameter table.") def assert_measurement_conditions_present_in_condition_table( From c04a75d49936e74e0de5caca577d35ffc795fd8c Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Thu, 19 May 2022 22:46:42 +0200 Subject: [PATCH 16/28] Use petab.Model in parameter mapping --- petab/models/model.py | 15 ++++++- petab/models/sbml_model.py | 9 +++- petab/parameter_mapping.py | 87 +++++++++++++++++++++++--------------- petab/problem.py | 15 +++---- 4 files changed, 82 insertions(+), 44 deletions(-) diff --git a/petab/models/model.py b/petab/models/model.py index 00270204..e8c24a50 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -60,8 +60,14 @@ def get_parameter_value(self, id_: str) -> float: ... @abc.abstractmethod - def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: - # TODO yet unused + def get_free_parameter_ids_with_values( + self + ) -> Iterable[Tuple[str, float]]: + """Get free model parameters along with their values + + Returns: + Iterator over tuples of (parameter_id, parameter_value) + """ ... @abc.abstractmethod @@ -123,6 +129,11 @@ def is_valid(self) -> bool: """ ... + @abc.abstractmethod + def is_state(self, id_: str) -> bool: + """Check whether the given ID corresponds to a model state""" + ... + def model_factory(filepath_or_buffer: Any, model_language: str) -> Model: """Create a PEtab model instance from the given model diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index bac5c675..b179fa29 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -82,7 +82,9 @@ def get_parameter_value(self, id_: str) -> float: raise ValueError(f"Parameter {id_} does not exist.") return parameter.getValue() - def get_parameter_ids_with_values(self) -> Iterable[Tuple[str, float]]: + def get_free_parameter_ids_with_values( + self + ) -> Iterable[Tuple[str, float]]: return ( (p.getId(), p.getValue()) for p in self.sbml_model.getListOfParameters() @@ -123,3 +125,8 @@ def is_valid(self) -> bool: valid = is_sbml_consistent(self.sbml_model.getSBMLDocument()) log_sbml_errors(self.sbml_model.getSBMLDocument()) return valid + + def is_state(self, id_: str) -> bool: + return (self.sbml_model.getSpecies(id_) is not None + or self.sbml_model.getCompartment(id_) is not None + or self.sbml_model.getRuleByVariable(id_) is not None) diff --git a/petab/parameter_mapping.py b/petab/parameter_mapping.py index 19f69d83..8347d99f 100644 --- a/petab/parameter_mapping.py +++ b/petab/parameter_mapping.py @@ -5,16 +5,17 @@ import numbers import os import re -from typing import Tuple, Dict, Union, Any, List, Optional, Iterable +import warnings +from typing import Any, Dict, Iterable, List, Optional, Tuple, Union import libsbml import numpy as np import pandas as pd -from . import lint, measurements, sbml, core, observables, parameters -from . import ENV_NUM_THREADS +from . import ENV_NUM_THREADS, core, lint, measurements, observables, \ + parameters from .C import * # noqa: F403 - +from .models import Model logger = logging.getLogger(__name__) __all__ = ['get_optimization_to_simulation_parameter_mapping', @@ -51,10 +52,11 @@ def get_optimization_to_simulation_parameter_mapping( warn_unmapped: Optional[bool] = True, scaled_parameters: bool = False, fill_fixed_parameters: bool = True, - allow_timepoint_specific_numeric_noise_parameters: bool = False + allow_timepoint_specific_numeric_noise_parameters: bool = False, + model: Model = None, ) -> List[ParMappingDictQuadruple]: """ - Create list of mapping dicts from PEtab-problem to SBML parameters. + Create list of mapping dicts from PEtab-problem to model parameters. Mapping can be performed in parallel. The number of threads is controlled by the environment variable with the name of @@ -64,8 +66,9 @@ def get_optimization_to_simulation_parameter_mapping( condition_df, measurement_df, parameter_df, observable_df: The dataframes in the PEtab format. sbml_model: - The sbml model with observables and noise specified according to - the PEtab format. + The SBML model (deprecated) + model: + The model. simulation_conditions: Table of simulation conditions as created by ``petab.get_simulation_conditions``. @@ -100,6 +103,16 @@ def get_optimization_to_simulation_parameter_mapping( If no preequilibration condition is defined, the respective dicts will be empty. ``NaN`` is used where no mapping exists. """ + if sbml_model: + warnings.warn("Passing a model via the `sbml_model` argument is " + "deprecated, use `model=petab.models.sbml_model." + "SbmlModel(...)` instead.", DeprecationWarning, + stacklevel=2) + from petab.models.sbml_model import SbmlModel + if model: + raise ValueError("Arguments `model` and `sbml_model` are " + "mutually exclusive.") + model = SbmlModel(sbml_model=sbml_model) # Ensure inputs are okay _perform_mapping_checks( @@ -111,12 +124,11 @@ def get_optimization_to_simulation_parameter_mapping( simulation_conditions = measurements.get_simulation_conditions( measurement_df) - simulation_parameters = sbml.get_model_parameters(sbml_model, - with_values=True) - # Add output parameters that are not already defined in the SBML model + simulation_parameters = dict(model.get_free_parameter_ids_with_values()) + # Add output parameters that are not already defined in the model if observable_df is not None: output_parameters = observables.get_output_parameters( - observable_df=observable_df, sbml_model=sbml_model) + observable_df=observable_df, model=model) for par_id in output_parameters: simulation_parameters[par_id] = np.nan @@ -129,7 +141,7 @@ def get_optimization_to_simulation_parameter_mapping( _map_condition, _map_condition_arg_packer( simulation_conditions, measurement_df, condition_df, - parameter_df, sbml_model, simulation_parameters, warn_unmapped, + parameter_df, model, simulation_parameters, warn_unmapped, scaled_parameters, fill_fixed_parameters, allow_timepoint_specific_numeric_noise_parameters)) return list(mapping) @@ -141,7 +153,7 @@ def get_optimization_to_simulation_parameter_mapping( _map_condition, _map_condition_arg_packer( simulation_conditions, measurement_df, condition_df, - parameter_df, sbml_model, simulation_parameters, warn_unmapped, + parameter_df, model, simulation_parameters, warn_unmapped, scaled_parameters, fill_fixed_parameters, allow_timepoint_specific_numeric_noise_parameters)) return list(mapping) @@ -152,7 +164,7 @@ def _map_condition_arg_packer( measurement_df, condition_df, parameter_df, - sbml_model, + model, simulation_parameters, warn_unmapped, scaled_parameters, @@ -162,7 +174,7 @@ def _map_condition_arg_packer( """Helper function to pack extra arguments for _map_condition""" for _, condition in simulation_conditions.iterrows(): yield(condition, measurement_df, condition_df, parameter_df, - sbml_model, simulation_parameters, warn_unmapped, + model, simulation_parameters, warn_unmapped, scaled_parameters, fill_fixed_parameters, allow_timepoint_specific_numeric_noise_parameters) @@ -174,7 +186,7 @@ def _map_condition(packed_args): :py:func:`get_optimization_to_simulation_parameter_mapping`. """ - (condition, measurement_df, condition_df, parameter_df, sbml_model, + (condition, measurement_df, condition_df, parameter_df, model, simulation_parameters, warn_unmapped, scaled_parameters, fill_fixed_parameters, allow_timepoint_specific_numeric_noise_parameters) = packed_args @@ -199,7 +211,7 @@ def _map_condition(packed_args): condition_id=condition[PREEQUILIBRATION_CONDITION_ID], is_preeq=True, cur_measurement_df=cur_measurement_df, - sbml_model=sbml_model, + model=model, condition_df=condition_df, parameter_df=parameter_df, simulation_parameters=simulation_parameters, @@ -214,7 +226,7 @@ def _map_condition(packed_args): condition_id=condition[SIMULATION_CONDITION_ID], is_preeq=False, cur_measurement_df=cur_measurement_df, - sbml_model=sbml_model, + model=model, condition_df=condition_df, parameter_df=parameter_df, simulation_parameters=simulation_parameters, @@ -232,14 +244,15 @@ def get_parameter_mapping_for_condition( condition_id: str, is_preeq: bool, cur_measurement_df: Optional[pd.DataFrame], - sbml_model: libsbml.Model, - condition_df: pd.DataFrame, + sbml_model: libsbml.Model = None, + condition_df: pd.DataFrame = None, parameter_df: pd.DataFrame = None, simulation_parameters: Optional[Dict[str, str]] = None, warn_unmapped: bool = True, scaled_parameters: bool = False, fill_fixed_parameters: bool = True, allow_timepoint_specific_numeric_noise_parameters: bool = False, + model: Model = None, ) -> Tuple[ParMappingDict, ScaleMappingDict]: """ Create dictionary of parameter value and parameter scale mappings from @@ -258,8 +271,9 @@ def get_parameter_mapping_for_condition( parameter_df: PEtab parameter DataFrame sbml_model: - The sbml model with observables and noise specified according to - the PEtab format used to retrieve simulation parameter IDs. + The SBML model (deprecated) + model: + The model. simulation_parameters: Model simulation parameter IDs mapped to parameter values (output of ``petab.sbml.get_model_parameters(.., with_values=True)``). @@ -286,6 +300,17 @@ def get_parameter_mapping_for_condition( Second dictionary mapping model parameter IDs to their scale. ``NaN`` is used where no mapping exists. """ + if sbml_model: + warnings.warn("Passing a model via the `sbml_model` argument is " + "deprecated, use `model=petab.models.sbml_model." + "SbmlModel(...)` instead.", DeprecationWarning, + stacklevel=2) + from petab.models.sbml_model import SbmlModel + if model: + raise ValueError("Arguments `model` and `sbml_model` are " + "mutually exclusive.") + model = SbmlModel(sbml_model=sbml_model) + if cur_measurement_df is not None: _perform_mapping_checks( cur_measurement_df, @@ -293,11 +318,11 @@ def get_parameter_mapping_for_condition( allow_timepoint_specific_numeric_noise_parameters) if simulation_parameters is None: - simulation_parameters = sbml.get_model_parameters(sbml_model, - with_values=True) + simulation_parameters = dict( + model.get_free_parameter_ids_with_values()) # NOTE: order matters here - the former is overwritten by the latter: - # SBML model < condition table < measurement < table parameter table + # model < condition table < measurement < table parameter table # initialize mapping dicts # for the case of matching simulation and optimization parameter vector @@ -314,7 +339,7 @@ def get_parameter_mapping_for_condition( handle_missing_overrides(par_mapping, warn=warn_unmapped) _apply_condition_parameters(par_mapping, scale_mapping, condition_id, - condition_df, sbml_model) + condition_df, model) _apply_parameter_table(par_mapping, scale_mapping, parameter_df, scaled_parameters, fill_fixed_parameters) @@ -386,7 +411,7 @@ def _apply_condition_parameters(par_mapping: ParMappingDict, scale_mapping: ScaleMappingDict, condition_id: str, condition_df: pd.DataFrame, - sbml_model: libsbml.Model) -> None: + model: Model) -> None: """Replace parameter IDs in parameter mapping dictionary by condition table parameter values (in-place). @@ -400,11 +425,7 @@ def _apply_condition_parameters(par_mapping: ParMappingDict, continue # Species, compartments, and rule targets are handled elsewhere - if sbml_model.getSpecies(overridee_id) is not None: - continue - if sbml_model.getCompartment(overridee_id) is not None: - continue - if sbml_model.getRuleByVariable(overridee_id) is not None: + if model.is_state(overridee_id): continue par_mapping[overridee_id] = core.to_float_if_float( diff --git a/petab/problem.py b/petab/problem.py index c950a6aa..7283bced 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -483,8 +483,7 @@ def get_optimization_parameter_scales(self): def get_model_parameters(self): """See :py:func:`petab.sbml.get_model_parameters`""" warn("petab.Problem.get_model_parameters is deprecated and will be " - "removed in a future version. Use " - "`petab.Problem.model.get_parameter_ids` instead.", + "removed in a future version.", DeprecationWarning, stacklevel=2) return sbml.get_model_parameters(self.sbml_model) @@ -706,13 +705,13 @@ def get_optimization_to_simulation_parameter_mapping( """ See get_simulation_to_optimization_parameter_mapping. """ - return parameter_mapping\ + return parameter_mapping \ .get_optimization_to_simulation_parameter_mapping( - self.condition_df, - self.measurement_df, - self.parameter_df, - self.observable_df, - self.sbml_model, + condition_df=self.condition_df, + measurement_df=self.measurement_df, + parameter_df=self.parameter_df, + observable_df=self.observable_df, + model=self.model, warn_unmapped=warn_unmapped, scaled_parameters=scaled_parameters, allow_timepoint_specific_numeric_noise_parameters= # noqa: E251,E501 From 69f826ea220d3756c3a154032056946b2fa2d004 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Fri, 20 May 2022 14:08:49 +0200 Subject: [PATCH 17/28] sbml_model->model --- tests/test_parameter_mapping.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_parameter_mapping.py b/tests/test_parameter_mapping.py index 52246964..5aaad22e 100644 --- a/tests/test_parameter_mapping.py +++ b/tests/test_parameter_mapping.py @@ -1,9 +1,11 @@ -import numpy as np import os + +import numpy as np import pandas as pd import petab -from petab.parameter_mapping import _apply_parameter_table from petab.C import * +from petab.models.sbml_model import SbmlModel +from petab.parameter_mapping import _apply_parameter_table # import fixtures pytest_plugins = [ @@ -61,8 +63,9 @@ def test_no_condition_specific(condition_df_2_conditions): 'fixedParameter1': LIN} )] + model = SbmlModel(sbml_model=ss_model.model) actual = petab.get_optimization_to_simulation_parameter_mapping( - sbml_model=ss_model.model, + model=model, measurement_df=measurement_df, condition_df=condition_df, ) @@ -101,7 +104,7 @@ def test_no_condition_specific(condition_df_2_conditions): ] actual = petab.get_optimization_to_simulation_parameter_mapping( - sbml_model=ss_model.model, + model=model, measurement_df=measurement_df, condition_df=condition_df, parameter_df=parameter_df @@ -135,7 +138,7 @@ def test_no_condition_specific(condition_df_2_conditions): ] actual = petab.get_optimization_to_simulation_parameter_mapping( - sbml_model=ss_model.model, + model=model, measurement_df=measurement_df, condition_df=condition_df, parameter_df=parameter_df, @@ -170,7 +173,7 @@ def test_no_condition_specific(condition_df_2_conditions): ] actual = petab.get_optimization_to_simulation_parameter_mapping( - sbml_model=ss_model.model, + model=model, measurement_df=measurement_df, condition_df=condition_df, parameter_df=parameter_df, @@ -188,6 +191,7 @@ def test_all_override(condition_df_2_conditions): ss_model = simplesbml.SbmlModel() ss_model.addParameter('dynamicParameter1', 0.0) ss_model.addParameter('dynamicParameter2', 0.0) + model = SbmlModel(sbml_model=ss_model.model) measurement_df = pd.DataFrame(data={ OBSERVABLE_ID: ['obs1', 'obs2', 'obs1', 'obs2'], @@ -253,7 +257,9 @@ def test_all_override(condition_df_2_conditions): actual = petab.get_optimization_to_simulation_parameter_mapping( measurement_df=measurement_df, condition_df=condition_df, - sbml_model=ss_model.model, parameter_df=parameter_df) + model=model, + parameter_df=parameter_df + ) assert actual == expected # For one case we test parallel execution, which must yield the same @@ -262,7 +268,9 @@ def test_all_override(condition_df_2_conditions): actual = petab.get_optimization_to_simulation_parameter_mapping( measurement_df=measurement_df, condition_df=condition_df, - sbml_model=ss_model.model, parameter_df=parameter_df) + model=model, + parameter_df=parameter_df + ) assert actual == expected @staticmethod From 9bc1e6f962628377b8fa221aaab99792c2039a2b Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Fri, 20 May 2022 14:26:00 +0200 Subject: [PATCH 18/28] cleanup --- petab/models/model.py | 25 ++----------------------- petab/models/sbml_model.py | 14 +------------- petab/parameters.py | 5 ++--- 3 files changed, 5 insertions(+), 39 deletions(-) diff --git a/petab/models/model.py b/petab/models/model.py index e8c24a50..a19919b8 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -15,7 +15,6 @@ def __init__(self): ... # TODO more coherent method names / arguments # TODO doc - # TODO remove unused @staticmethod @abc.abstractmethod @@ -41,14 +40,6 @@ def to_file(self, filename: [str, Path]): def type_id(cls): ... - @abc.abstractmethod - def get_parameter_ids(self) -> Iterable[str]: - """Get all parameter IDs from this model - - :returns: Iterator over model parameter IDs - """ - ... - @abc.abstractmethod def get_parameter_value(self, id_: str) -> float: """Get a parameter value @@ -70,16 +61,6 @@ def get_free_parameter_ids_with_values( """ ... - @abc.abstractmethod - def has_species_with_id(self, entity_id: str) -> bool: - # TODO yet unused - ... - - @abc.abstractmethod - def has_compartment_with_id(self, entity_id: str) -> bool: - # TODO yet unused - ... - @abc.abstractmethod def has_entity_with_id(self, entity_id) -> bool: """Check if there is a model entity with the given ID @@ -110,9 +91,8 @@ def get_valid_ids_for_condition_table(self) -> Iterable[str]: @abc.abstractmethod def symbol_allowed_in_observable_formula(self, id_: str) -> bool: - """Check if the given ID is allowed to be used in observable formulas - - # TODO currently also used for noise + """Check if the given ID is allowed to be used in observable and noise + formulas :returns: ``True``, if allowed, ``False`` otherwise """ @@ -123,7 +103,6 @@ def symbol_allowed_in_observable_formula(self, id_: str) -> bool: def is_valid(self) -> bool: """Validate this model - # TODO optional printing? :returns: `True` if the model is valid, `False` if there are errors in this model """ diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index b179fa29..9582123e 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -70,12 +70,6 @@ def to_file(self, filename: [str, Path]): write_sbml(self.sbml_document or self.sbml_model.getSBMLDocument(), filename) - def get_parameter_ids(self) -> Iterable[str]: - return ( - p.getId() for p in self.sbml_model.getListOfParameters() - if self.sbml_model.getAssignmentRuleByVariable(p.getId()) is None - ) - def get_parameter_value(self, id_: str) -> float: parameter = self.sbml_model.getParameter(id_) if not parameter: @@ -91,17 +85,11 @@ def get_free_parameter_ids_with_values( if self.sbml_model.getAssignmentRuleByVariable(p.getId()) is None ) - def has_species_with_id(self, entity_id: str) -> bool: - return self.sbml_model.getSpecies(entity_id) is not None - - def has_compartment_with_id(self, entity_id: str) -> bool: - return self.sbml_model.getCompartment(entity_id) is not None - def has_entity_with_id(self, entity_id) -> bool: return self.sbml_model.getElementBySId(entity_id) is not None def get_valid_parameters_for_parameter_table(self) -> Iterable[str]: - # exclude rule targets + # All parameters except rule-targets disallowed_set = { ar.getVariable() for ar in self.sbml_model.getListOfRules() } diff --git a/petab/parameters.py b/petab/parameters.py index af514f64..1b2b61b4 100644 --- a/petab/parameters.py +++ b/petab/parameters.py @@ -243,7 +243,6 @@ def append_overrides(overrides): row.get(NOISE_PARAMETERS, None))) # Add output parameters except for placeholders - model_parameter_ids = set(model.get_parameter_ids()) for kwargs in [dict(observables=True, noise=False), dict(observables=False, noise=True)]: output_parameters = observables.get_output_parameters( @@ -251,13 +250,13 @@ def append_overrides(overrides): placeholders = observables.get_placeholders( observable_df, **kwargs) for p in output_parameters: - if p not in placeholders and p not in model_parameter_ids: + if p not in placeholders: parameter_ids[p] = None # Add condition table parametric overrides unless already defined in the # model for p in conditions.get_parametric_overrides(condition_df): - if p not in model_parameter_ids: + if not model.has_entity_with_id(p): parameter_ids[p] = None return parameter_ids.keys() From d540c47e731bbc9251f82d916f61f40d98a9d583 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Tue, 24 May 2022 17:08:05 +0200 Subject: [PATCH 19/28] fixup merge --- petab/parameter_mapping.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/petab/parameter_mapping.py b/petab/parameter_mapping.py index 3345d3fb..c6777eee 100644 --- a/petab/parameter_mapping.py +++ b/petab/parameter_mapping.py @@ -435,14 +435,14 @@ def _apply_condition_parameters(par_mapping: ParMappingDict, and np.isnan(par_mapping[overridee_id]): # NaN in the condition table for an entity without time derivative # indicates that the model value should be used - parameter = sbml_model.getParameter(overridee_id) - if parameter: - par_mapping[overridee_id] = parameter.getValue() - else: + try: + par_mapping[overridee_id] = \ + model.get_parameter_value(overridee_id) + except ValueError as e: raise NotImplementedError( "Not sure how to handle NaN in condition table for " f"{overridee_id}." - ) + ) from e scale_mapping[overridee_id] = LIN From bb61a12ae3c22157b38d5b35864493d284e1d5d8 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 25 May 2022 10:43:06 +0200 Subject: [PATCH 20/28] fix merge --- petab/problem.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/petab/problem.py b/petab/problem.py index ef9fc196..9d258f82 100644 --- a/petab/problem.py +++ b/petab/problem.py @@ -132,7 +132,8 @@ def from_files( model = model_factory(sbml_file, MODEL_TYPE_SBML) \ if sbml_file else None - condition_df = conditions.get_condition_df(condition_file) \ + condition_df = core.concat_tables( + condition_file, conditions.get_condition_df) \ if condition_file else None # If there are multiple tables, we will merge them @@ -229,20 +230,19 @@ def from_yaml(yaml_config: Union[Dict, Path, str]) -> 'Problem': MODEL_TYPE_SBML) \ if problem0[SBML_FILES] else None - if problem0[MEASUREMENT_FILES]: - measurement_files = [ - get_path(f) for f in problem0[MEASUREMENT_FILES] - ] - # If there are multiple tables, we will merge them - measurement_df = core.concat_tables( - measurement_files, measurements.get_measurement_df) \ - if measurement_files else None - else: - measurement_df = None + measurement_files = [ + get_path(f) for f in problem0.get(MEASUREMENT_FILES, [])] + # If there are multiple tables, we will merge them + measurement_df = core.concat_tables( + measurement_files, measurements.get_measurement_df) \ + if measurement_files else None - condition_df = core.get_condition_df( - [get_path(f) for f in problem0[CONDITION_FILES]] - ) + condition_files = [ + get_path(f) for f in problem0.get(CONDITION_FILES, [])] + # If there are multiple tables, we will merge them + condition_df = core.concat_tables( + condition_files, conditions.get_condition_df) \ + if condition_files else None visualization_files = [ get_path(f) for f in problem0.get(VISUALIZATION_FILES, [])] @@ -265,7 +265,6 @@ def from_yaml(yaml_config: Union[Dict, Path, str]) -> 'Problem': model=model, visualization_df=visualization_df) - @staticmethod def from_combine(filename: Union[Path, str]) -> 'Problem': """Read PEtab COMBINE archive (http://co.mbine.org/documents/archive). From ed7c23c5a7da16f23f5a2337eee70d6219d6da1d Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:15:00 +0200 Subject: [PATCH 21/28] is/are --- petab/lint.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/petab/lint.py b/petab/lint.py index 660f045e..f6eee29f 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -874,7 +874,8 @@ def assert_model_parameters_in_condition_or_parameter_table( if model.has_entity_with_id(x) } if disallowed_in_condition: - raise AssertionError(f"{disallowed_in_condition} is not " + is_or_are = "is" if len(disallowed_in_condition) == 1 else "are" + raise AssertionError(f"{disallowed_in_condition} {is_or_are} not " "allowed to occur in condition table " "columns.") @@ -885,13 +886,15 @@ def assert_model_parameters_in_condition_or_parameter_table( } if disallowed_in_parameters: - raise AssertionError(f"{disallowed_in_parameters} is not " + is_or_are = "is" if len(disallowed_in_parameters) == 1 else "are" + raise AssertionError(f"{disallowed_in_parameters} {is_or_are} not " "allowed to occur in the parameters table.") in_both = entities_in_condition_table & entities_in_parameter_table if in_both: - raise AssertionError(f"{in_both} are present in both condition table " - "and parameter table.") + is_or_are = "is" if len(in_both) == 1 else "are" + raise AssertionError(f"{in_both} {is_or_are} present in both " + "condition table and parameter table.") def assert_measurement_conditions_present_in_condition_table( From 4dbcfe0967bbe6531ff5d086a6a9d8c6054728ce Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:19:02 +0200 Subject: [PATCH 22/28] Update petab/models/model.py Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> --- petab/models/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/petab/models/model.py b/petab/models/model.py index a19919b8..b5075656 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -8,7 +8,7 @@ from . import MODEL_TYPE_SBML, known_model_types -class Model: +class Model(abc.ABC): """Base class for wrappers for any PEtab-supported model type""" def __init__(self): From 804188bc98615f665cd38b2af48105522a34438f Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:20:25 +0200 Subject: [PATCH 23/28] Update petab/models/model.py Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com> --- petab/models/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/petab/models/model.py b/petab/models/model.py index b5075656..48796f05 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -109,8 +109,8 @@ def is_valid(self) -> bool: ... @abc.abstractmethod - def is_state(self, id_: str) -> bool: - """Check whether the given ID corresponds to a model state""" + def is_state_variable(self, id_: str) -> bool: + """Check whether the given ID corresponds to a model state variable""" ... From 3b337b07c47c13a151076665f52684e8f98193b1 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:15:38 +0200 Subject: [PATCH 24/28] the --- petab/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/petab/lint.py b/petab/lint.py index f6eee29f..822edfeb 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -894,7 +894,7 @@ def assert_model_parameters_in_condition_or_parameter_table( if in_both: is_or_are = "is" if len(in_both) == 1 else "are" raise AssertionError(f"{in_both} {is_or_are} present in both " - "condition table and parameter table.") + "the condition table and the parameter table.") def assert_measurement_conditions_present_in_condition_table( From d66e18a9e5ccc2f71f87a942794898e18e39af14 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:22:52 +0200 Subject: [PATCH 25/28] import --- petab/models/sbml_model.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 9582123e..5df76945 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -9,7 +9,7 @@ from . import MODEL_TYPE_SBML from .model import Model from ..sbml import (get_sbml_model, is_sbml_consistent, load_sbml_from_string, - log_sbml_errors) + log_sbml_errors, write_sbml) class SbmlModel(Model): @@ -66,7 +66,6 @@ def from_file(filepath_or_buffer): ) def to_file(self, filename: [str, Path]): - from ..sbml import write_sbml write_sbml(self.sbml_document or self.sbml_model.getSBMLDocument(), filename) From 5464928809505ed59a8f7205ff923b15e3550b1b Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:26:32 +0200 Subject: [PATCH 26/28] fixup rename --- petab/models/sbml_model.py | 2 +- petab/parameter_mapping.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/petab/models/sbml_model.py b/petab/models/sbml_model.py index 5df76945..3dcabe2f 100644 --- a/petab/models/sbml_model.py +++ b/petab/models/sbml_model.py @@ -113,7 +113,7 @@ def is_valid(self) -> bool: log_sbml_errors(self.sbml_model.getSBMLDocument()) return valid - def is_state(self, id_: str) -> bool: + def is_state_variable(self, id_: str) -> bool: return (self.sbml_model.getSpecies(id_) is not None or self.sbml_model.getCompartment(id_) is not None or self.sbml_model.getRuleByVariable(id_) is not None) diff --git a/petab/parameter_mapping.py b/petab/parameter_mapping.py index c6777eee..6a309011 100644 --- a/petab/parameter_mapping.py +++ b/petab/parameter_mapping.py @@ -425,7 +425,7 @@ def _apply_condition_parameters(par_mapping: ParMappingDict, continue # Species, compartments, and rule targets are handled elsewhere - if model.is_state(overridee_id): + if model.is_state_variable(overridee_id): continue par_mapping[overridee_id] = core.to_float_if_float( From 4dcf1f626dd0da5f4068d14c9530c40b9159c04e Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Mon, 20 Jun 2022 16:31:10 +0200 Subject: [PATCH 27/28] PARAMETER_SEPARATOR --- petab/C.py | 3 +++ petab/core.py | 4 ++-- petab/lint.py | 8 +++++--- petab/measurements.py | 2 +- petab/parameters.py | 8 +++++--- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/petab/C.py b/petab/C.py index ea56cf33..8e63f00f 100644 --- a/petab/C.py +++ b/petab/C.py @@ -274,3 +274,6 @@ RESIDUAL = 'residual' #: NOISE_VALUE = 'noiseValue' + +# separator for multiple parameter values (bounds, observableParameters, ...) +PARAMETER_SEPARATOR = ';' diff --git a/petab/core.py b/petab/core.py index bfab25d5..30256ca6 100644 --- a/petab/core.py +++ b/petab/core.py @@ -120,8 +120,8 @@ def flatten_timepoint_specific_output_overrides( replacement_id = '' for field in possible_groupvars: if field in groupvars: - val = str(groupvar[groupvars.index(field) - ]).replace(';', '_').replace('.', '_') + val = str(groupvar[groupvars.index(field)])\ + .replace(PARAMETER_SEPARATOR, '_').replace('.', '_') if replacement_id == '': replacement_id = val elif val != '': diff --git a/petab/lint.py b/petab/lint.py index 822edfeb..e4e9c683 100644 --- a/petab/lint.py +++ b/petab/lint.py @@ -537,7 +537,9 @@ def assert_parameter_prior_parameters_are_valid( continue # parse parameters try: - pars = tuple([float(val) for val in pars_str.split(';')]) + pars = tuple( + float(val) for val in pars_str.split(PARAMETER_SEPARATOR) + ) except ValueError: raise AssertionError( f"Could not parse prior parameters '{pars_str}'.") @@ -545,8 +547,8 @@ def assert_parameter_prior_parameters_are_valid( if len(pars) != 2: raise AssertionError( f"The prior parameters '{pars}' do not contain the " - "expected number of entries (currently 'par1;par2' " - "for all prior types).") + "expected number of entries (currently 'par1" + f"{PARAMETER_SEPARATOR}par2' for all prior types).") def assert_parameter_estimate_is_boolean(parameter_df: pd.DataFrame) -> None: diff --git a/petab/measurements.py b/petab/measurements.py index d7e1c954..72cc2ce0 100644 --- a/petab/measurements.py +++ b/petab/measurements.py @@ -154,7 +154,7 @@ def get_unique_parameters(series): def split_parameter_replacement_list( list_string: Union[str, numbers.Number], - delim: str = ';') -> List[Union[str, numbers.Number]]: + delim: str = PARAMETER_SEPARATOR) -> List[Union[str, numbers.Number]]: """ Split values in observableParameters and noiseParameters in measurement table. diff --git a/petab/parameters.py b/petab/parameters.py index 3aaafb2e..3600e163 100644 --- a/petab/parameters.py +++ b/petab/parameters.py @@ -360,8 +360,10 @@ def get_priors_from_df(parameter_df: pd.DataFrame, if core.is_empty(pars_str): lb, ub = map_scale([row[LOWER_BOUND], row[UPPER_BOUND]], [row[PARAMETER_SCALE]] * 2) - pars_str = f'{lb};{ub}' - prior_pars = tuple(float(entry) for entry in pars_str.split(';')) + pars_str = f'{lb}{PARAMETER_SEPARATOR}{ub}' + prior_pars = tuple( + float(entry) for entry in pars_str.split(PARAMETER_SEPARATOR) + ) # add parameter scale and bounds, as this may be needed par_scale = row[PARAMETER_SCALE] @@ -490,6 +492,6 @@ def normalize_parameter_df(parameter_df: pd.DataFrame) -> pd.DataFrame: and row[prior_type_col] == PARAMETER_SCALE_UNIFORM: lb, ub = map_scale([row[LOWER_BOUND], row[UPPER_BOUND]], [row[PARAMETER_SCALE]] * 2) - df.loc[irow, prior_par_col] = f'{lb};{ub}' + df.loc[irow, prior_par_col] = f'{lb}{PARAMETER_SEPARATOR}{ub}' return df From 49e518c07a72cb6d3ac0c45b025d423944d6c9a5 Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Tue, 21 Jun 2022 14:10:56 +0200 Subject: [PATCH 28/28] .. --- petab/models/model.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/petab/models/model.py b/petab/models/model.py index 48796f05..ae2ad3b8 100644 --- a/petab/models/model.py +++ b/petab/models/model.py @@ -13,8 +13,6 @@ class Model(abc.ABC): def __init__(self): ... - # TODO more coherent method names / arguments - # TODO doc @staticmethod @abc.abstractmethod