diff --git a/petab/v2/core.py b/petab/v2/core.py index fb206502..2bf6a49e 100644 --- a/petab/v2/core.py +++ b/petab/v2/core.py @@ -19,6 +19,7 @@ Annotated, Any, Generic, + Literal, Self, TypeVar, get_args, @@ -318,9 +319,9 @@ class Observable(BaseModel): #: Observable name. name: str | None = Field(alias=C.OBSERVABLE_NAME, default=None) #: Observable formula. - formula: sp.Basic | None = Field(alias=C.OBSERVABLE_FORMULA, default=None) + formula: sp.Basic = Field(alias=C.OBSERVABLE_FORMULA) #: Noise formula. - noise_formula: sp.Basic | None = Field(alias=C.NOISE_FORMULA, default=None) + noise_formula: sp.Basic = Field(alias=C.NOISE_FORMULA) #: Noise distribution. noise_distribution: NoiseDistribution = Field( alias=C.NOISE_DISTRIBUTION, default=NoiseDistribution.NORMAL @@ -926,7 +927,7 @@ class Parameter(BaseModel): ) #: Nominal value. nominal_value: Annotated[ - float | None, BeforeValidator(_convert_nan_to_none) + float | Literal["array"] | None, BeforeValidator(_convert_nan_to_none) ] = Field(alias=C.NOMINAL_VALUE, default=None) #: Is the parameter to be estimated? estimate: bool = Field(alias=C.ESTIMATE, default=True) diff --git a/petab/v2/lint.py b/petab/v2/lint.py index 687d58f2..da3b8a92 100644 --- a/petab/v2/lint.py +++ b/petab/v2/lint.py @@ -44,6 +44,7 @@ "CheckPriorDistribution", "CheckUndefinedExperiments", "CheckInitialChangeSymbols", + "CheckMappingTable", "lint_problem", "default_validation_tasks", ] @@ -445,7 +446,7 @@ def run(self, problem: Problem) -> ValidationIssue | None: # check for uniqueness of all primary keys counter = Counter(c.id for c in problem.conditions) - duplicates = {id_ for id_, count in counter.items() if count > 1} + duplicates = sorted(id_ for id_, count in counter.items() if count > 1) if duplicates: return ValidationError( @@ -453,7 +454,7 @@ def run(self, problem: Problem) -> ValidationIssue | None: ) counter = Counter(o.id for o in problem.observables) - duplicates = {id_ for id_, count in counter.items() if count > 1} + duplicates = sorted(id_ for id_, count in counter.items() if count > 1) if duplicates: return ValidationError( @@ -461,7 +462,7 @@ def run(self, problem: Problem) -> ValidationIssue | None: ) counter = Counter(e.id for e in problem.experiments) - duplicates = {id_ for id_, count in counter.items() if count > 1} + duplicates = sorted(id_ for id_, count in counter.items() if count > 1) if duplicates: return ValidationError( @@ -469,7 +470,7 @@ def run(self, problem: Problem) -> ValidationIssue | None: ) counter = Counter(p.id for p in problem.parameters) - duplicates = {id_ for id_, count in counter.items() if count > 1} + duplicates = sorted(id_ for id_, count in counter.items() if count > 1) if duplicates: return ValidationError( @@ -508,7 +509,11 @@ def run(self, problem: Problem) -> ValidationIssue | None: for experiment in problem.experiments: # Check that there are no duplicate timepoints counter = Counter(period.time for period in experiment.periods) - duplicates = {time for time, count in counter.items() if count > 1} + duplicates = sorted( + time + for time, count in counter.items() + if count > 1 + ) if duplicates: messages.append( f"Experiment {experiment.id} contains duplicate " @@ -551,7 +556,8 @@ def run(self, problem: Problem) -> ValidationIssue | None: class CheckAllParametersPresentInParameterTable(ValidationTask): """Ensure all required parameters are contained in the parameter table - with no additional ones.""" + with no additional ones. This also ensures that the mapping table petab ids + are used in the PEtab problem.""" def run(self, problem: Problem) -> ValidationIssue | None: if problem.model is None: @@ -825,8 +831,8 @@ def run(self, problem: Problem) -> ValidationIssue | None: if parameter.prior_distribution not in PRIOR_DISTRIBUTIONS: messages.append( - f"Prior distribution `{parameter.prior_distribution}' " - f"for parameter `{parameter.id}' is not valid." + f"Prior distribution `{parameter.prior_distribution}` " + f"for parameter `{parameter.id}` is not valid." ) continue @@ -834,8 +840,8 @@ def run(self, problem: Problem) -> ValidationIssue | None: exp_num_par := self._num_pars[parameter.prior_distribution] ) != len(parameter.prior_parameters): messages.append( - f"Prior distribution `{parameter.prior_distribution}' " - f"for parameter `{parameter.id}' requires " + f"Prior distribution `{parameter.prior_distribution}` " + f"for parameter `{parameter.id}` requires " f"{exp_num_par} parameters, but got " f"{len(parameter.prior_parameters)} " f"({parameter.prior_parameters})." @@ -848,8 +854,8 @@ def run(self, problem: Problem) -> ValidationIssue | None: _ = parameter.prior_dist.sample(1) except Exception as e: messages.append( - f"Prior parameters `{parameter.prior_parameters}' " - f"for parameter `{parameter.id}' are invalid " + f"Prior parameters `{parameter.prior_parameters}` " + f"for parameter `{parameter.id}` are invalid " f"(hint: {e})." ) @@ -874,7 +880,7 @@ def run(self, problem: Problem) -> ValidationIssue | None: continue messages.append( - f"Measurement `{measurement}' does not have a model ID, " + f"Measurement `{measurement}` does not have a model ID, " "but there are multiple models available. " "Please specify the model ID in the measurement table." ) @@ -882,8 +888,8 @@ def run(self, problem: Problem) -> ValidationIssue | None: if measurement.model_id not in available_models: messages.append( - f"Measurement `{measurement}' has model ID " - f"`{measurement.model_id}' which does not match " + f"Measurement `{measurement}` has model ID " + f"`{measurement.model_id}` which does not match " "any of the available models: " f"{available_models}." ) @@ -894,6 +900,78 @@ def run(self, problem: Problem) -> ValidationIssue | None: return None +class CheckMappingTable(ValidationTask): + """Validate the mapping table.""" + + def run(self, problem: Problem) -> ValidationIssue | None: + messages = [] + + # Mapping table is optional + if problem.mappings: + # Check that each id, across both the petabEntityId and + # modelEntityId columns, occurs only once + must_be_unique_ids = [] + for mapping in problem.mappings: + petab_id := getattr(mapping, "petab_id"): + model_id := getattr(mapping, "model_id"): + + if petab_id: + must_be_unique_ids.append(petab_id) + # Duplicates for annotation-only rows (identity mappings) + # are permitted. + if petab_id == model_id: + continue + if model_id: + must_be_unique_ids.append(model_id) + + non_unique_ids = sorted( + id_ + for id_, count in Counter(must_be_unique_ids).items() + if count > 1 + ) + if non_unique_ids: + return ValidationError( + f"Mapping table contains non-unique IDs: {non_unique_ids}." + ) + + # petabEntityId is not defined elsewhere in the PEtab problem + new_petab_ids = { + m.petab_id + for m in problem.mappings + # Ignore identity mappings used for annotation + if m.petab_id != m.model_id + } + old_petab_ids = ( + {c.id for c in problem.conditions} + | {e.id for e in problem.experiments} + | {o.id for o in problem.observables} + ) + if overdefined_ids := sorted(new_petab_ids & old_petab_ids): + messages.append( + f"PEtab IDs `{overdefined_ids}` are " + "defined in the mapping table but also defined through " + "other PEtab tables." + ) + + for mapping in problem.mappings: + # petabEntityId not referenced in any model, if alias + for model in problem.models: + if ( + mapping.petab_id != mapping.model_id + and model.has_entity_with_id(mapping.petab_id) + ): + messages.append( + f"`{mapping.petab_id}` is used in the mapping " + "table and referenced directly in the model " + f"`{model.model_id}`." + ) + + if messages: + return ValidationError("\n".join(messages)) + + return None + + def get_valid_parameters_for_parameter_table( problem: Problem, ) -> set[str]: @@ -933,9 +1011,16 @@ def get_valid_parameters_for_parameter_table( if p not in invalid ) + # Add petab ids from mapping table if they are used for aliasing + # FIXME only add mapping.petab_id to allowed parameter IDs list if it + # aliases an invalid PEtab ID? See + # https://github.com/PEtab-dev/libpetab-python/pull/482#discussion_r3420762034 for mapping in problem.mappings: if mapping.model_id and mapping.model_id in parameter_ids.keys(): parameter_ids[mapping.petab_id] = None + # An aliased model id is not a valid parameter id + if mapping.model_id in parameter_ids: + del parameter_ids[mapping.model_id] # add output parameters from observable table output_parameters = problem.get_output_parameters() @@ -977,20 +1062,13 @@ def get_required_parameters_for_parameter_table( measurement table as well as all parametric condition table overrides that are not defined in the model. """ - parameter_ids = set() - condition_targets = { - change.target_id - for cond in problem.conditions - for change in cond.changes - } + # Start with mapping table petab ids + parameter_ids = {m.petab_id for m in problem.mappings} # Add parameters from measurement table, unless they are fixed parameters def append_overrides(overrides): parameter_ids.update( - str_p - for p in overrides - if isinstance(p, sp.Symbol) - and (str_p := str(p)) not in condition_targets + str(p) for p in overrides if isinstance(p, sp.Symbol) ) for m in problem.measurements: @@ -1033,7 +1111,12 @@ def append_overrides(overrides): if not problem.model.has_entity_with_id(str(p)) ) - # parameters that are overridden via the condition table are not allowed + # Parameters that are overridden via the condition table are not allowed + condition_targets = { + change.target_id + for cond in problem.conditions + for change in cond.changes + } parameter_ids -= condition_targets return parameter_ids @@ -1090,5 +1173,5 @@ def get_placeholders( CheckUnusedConditions(), CheckPriorDistribution(), CheckInitialChangeSymbols(), - # TODO validate mapping table + CheckMappingTable(), ] diff --git a/tests/v2/test_core.py b/tests/v2/test_core.py index 22dbf0e1..7b5ad420 100644 --- a/tests/v2/test_core.py +++ b/tests/v2/test_core.py @@ -181,7 +181,7 @@ def test_measurments(): def test_observable(): - Observable(id="obs1", formula=x + y) + Observable(id="obs1", formula=x + y, noiseFormula=1) Observable(id="obs1", formula="x + y", noise_formula="x + y") Observable(id="obs1", formula=1, noise_formula=2) Observable( @@ -198,9 +198,17 @@ def test_observable(): observable_parameters=[sp.Symbol("p1")], noise_parameters=[sp.Symbol("n1")], ) - assert Observable(id="obs1", formula="x + y", non_petab=1).non_petab == 1 + assert ( + Observable( + id="obs1", + formula="x + y", + noise_formula="x + y", + non_petab=1, + ).non_petab + == 1 + ) - o = Observable(id="obs1", formula=x + y) + o = Observable(id="obs1", formula=x + y, noise_formula=1) assert o.observable_placeholders == [] assert o.noise_placeholders == [] @@ -492,14 +500,14 @@ def test_modify_problem(): problem.condition_df, exp_condition_df, check_dtype=False ) - problem.add_observable("observable1", "1") + problem.add_observable("observable1", "1", noise_formula=1) problem.add_observable("observable2", "2", noise_formula=2.2) exp_observable_df = pd.DataFrame( data={ OBSERVABLE_ID: ["observable1", "observable2"], OBSERVABLE_FORMULA: [1, 2], - NOISE_FORMULA: [np.nan, 2.2], + NOISE_FORMULA: [1, 2.2], } ).set_index([OBSERVABLE_ID]) assert_frame_equal( diff --git a/tests/v2/test_lint.py b/tests/v2/test_lint.py index 7eb6dc91..a101ee64 100644 --- a/tests/v2/test_lint.py +++ b/tests/v2/test_lint.py @@ -43,7 +43,7 @@ def test_invalid_model_id_in_measurements(): """Test that measurements with an invalid model ID are caught.""" problem = Problem() problem.models.append(SbmlModel.from_antimony("p1 = 1", model_id="model1")) - problem.add_observable("obs1", "A") + problem.add_observable("obs1", "A", 1) problem.add_measurement("obs1", experiment_id="e1", time=0, measurement=1) check = CheckMeasurementModelId() @@ -70,7 +70,7 @@ def test_undefined_experiment_id_in_measurements(): """Test that measurements with an undefined experiment ID are caught.""" problem = Problem() problem.add_experiment("e1", 0, "c1") - problem.add_observable("obs1", "A") + problem.add_observable("obs1", "A", 1) problem.add_measurement("obs1", experiment_id="e1", time=0, measurement=1) check = CheckUndefinedExperiments() @@ -107,3 +107,43 @@ def test_validate_initial_change_symbols(): problem.parameter_tables[0].parameters.remove(problem["p2"]) assert (error := check.run(problem)) is not None assert "contains additional symbols: {'p2'}" in error.message + + +def test_check_mapping_table(): + """Test checks related to the mapping table.""" + problem = Problem() + # FIXME see https://github.com/PEtab-dev/libpetab-python/pull/482#discussion_r3431330125 + problem.model = SbmlModel.from_antimony("a.mean = 1") + problem.add_mapping( + petab_id="a_m", + model_id="a.mean", + name=None, + ) + problem.add_parameter( + "a_m", + estimate=True, + nominal_value=2, + lb=0, + ub=10, + ) + + check = CheckMappingTable() + assert check.run(problem) is None + + check = CheckAllParametersPresentInParameterTable() + assert check.run(problem) is None + + # add a petab id without model id but with name for annotation + problem.add_mapping(petab_id="p2", model_id=None, name="Parameter 2") + problem.add_parameter("p2", estimate=True, nominal_value=1, lb=0, ub=10) + + check = CheckMappingTable() + assert check.run(problem) is None + + # Invalid: petabEntityId is referenced in the model + problem.model = SbmlModel.from_antimony("a.mean = 1; a_m = 2") + assert (error := check.run(problem)) is not None + assert ( + "`a_m` is used in the mapping table and referenced directly" + in error.message + )