Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add abstraction for (SBML) models #133

Merged
merged 32 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6b26c69
Add abstraction for models
dweindl Apr 26, 2022
796d042
Use new Model in petab.Problem
dweindl Apr 27, 2022
796219b
__init__.py
dweindl Apr 27, 2022
d200396
fix merge
dweindl Apr 28, 2022
52f0ec0
TYPE_CHECKING
dweindl Apr 28, 2022
0736225
Replace most sbml_model occurrences by new Model
dweindl Apr 29, 2022
d798652
doc
dweindl May 4, 2022
9a34c74
..
dweindl May 4, 2022
e0b6e74
Deprecate petab.Problem.from_files
dweindl May 5, 2022
2924c2c
Update petab.Problem to use Model
dweindl May 5, 2022
507c196
Update to test_petab
dweindl May 5, 2022
7d9dae8
Add model type constants; add type code to class; add list of known t…
dweindl May 5, 2022
3d4cda5
Model.to_file
dweindl May 5, 2022
d8d1774
fixup, doc
dweindl May 19, 2022
ee977a7
cleanup assert_model_parameters_in_condition_or_parameter_table
dweindl May 19, 2022
c04a75d
Use petab.Model in parameter mapping
dweindl May 19, 2022
69f826e
sbml_model->model
dweindl May 20, 2022
9bc1e6f
cleanup
dweindl May 20, 2022
4996699
Merge branch 'develop' into abstract_model
dweindl May 24, 2022
d540c47
fixup merge
dweindl May 24, 2022
78251a0
Merge branch 'develop' into abstract_model
dweindl May 25, 2022
bb61a12
fix merge
dweindl May 25, 2022
66e1bc4
Merge branch 'develop' into abstract_model
dweindl May 25, 2022
ed7c23c
is/are
dweindl Jun 20, 2022
4dbcfe0
Update petab/models/model.py
dweindl Jun 20, 2022
804188b
Update petab/models/model.py
dweindl Jun 20, 2022
3b337b0
the
dweindl Jun 20, 2022
d66e18a
import
dweindl Jun 20, 2022
5464928
fixup rename
dweindl Jun 20, 2022
4dcf1f6
PARAMETER_SEPARATOR
dweindl Jun 20, 2022
a9bf397
Merge branch 'develop' into abstract_model
dweindl Jun 20, 2022
49e518c
..
dweindl Jun 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/example/example_visualization.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -264,4 +264,4 @@
},
"nbformat": 4,
"nbformat_minor": 2
}
}
2 changes: 1 addition & 1 deletion doc/example/example_visualization_without_visspec.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,4 @@
},
"nbformat": 4,
"nbformat_minor": 2
}
}
3 changes: 3 additions & 0 deletions petab/C.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,6 @@
RESIDUAL = 'residual'
#:
NOISE_VALUE = 'noiseValue'

# separator for multiple parameter values (bounds, observableParameters, ...)
PARAMETER_SEPARATOR = ';'
4 changes: 2 additions & 2 deletions petab/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 != '':
Expand Down
134 changes: 70 additions & 64 deletions petab/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -189,15 +187,15 @@ 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:
"""Run sanity checks on PEtab parameter table

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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -539,16 +537,18 @@ 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}'.")
# all distributions take 2 parameters
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:
Expand Down Expand Up @@ -764,13 +764,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 |= not problem.model.is_valid()
dweindl marked this conversation as resolved.
Show resolved Hide resolved
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...")
Expand All @@ -790,7 +788,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
Expand All @@ -804,9 +802,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
Expand All @@ -816,7 +814,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:
Expand All @@ -825,11 +823,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
)
Expand All @@ -840,7 +838,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.')
Expand All @@ -851,46 +849,54 @@ 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()

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_parameter_df and in_condition_df:
raise AssertionError(f"Model parameter '{parameter_id}' present "
"in both condition table and parameter "
"table.")
allowed_in_condition_cols = set(model.get_valid_ids_for_condition_table())
allowed_in_parameter_table = \
set(model.get_valid_parameters_for_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:
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.")

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:
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:
is_or_are = "is" if len(in_both) == 1 else "are"
raise AssertionError(f"{in_both} {is_or_are} present in both "
"the condition table and the parameter table.")


def assert_measurement_conditions_present_in_condition_table(
Expand Down
2 changes: 1 addition & 1 deletion petab/measurements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it ever make sense to pass anything but PARAMETER_SEPARATOR here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not for now. If the separator ever changes, it can be useful, but shouldn't hurt keeping the optional argument.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why is it optional for this specific function (in contrast to all the other code, where it isn't optional)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because adding it to the whole potential call stack will be a mess 🙈

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

"""
Split values in observableParameters and noiseParameters in measurement
table.
Expand Down
5 changes: 5 additions & 0 deletions petab/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
MODEL_TYPE_SBML = 'sbml'

known_model_types = {MODEL_TYPE_SBML}
Comment on lines +1 to +3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be here or the constants file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer keeping it more modular, but could consider importing it in petab.C


from .model import Model # noqa F401
Loading