Support PEtab SciML problems and enable linting by m-philipps · Pull Request #482 · PEtab-dev/libpetab-python

m-philipps · 2026-05-26T22:26:39Z

WIP

codecov-commenter · 2026-06-11T15:02:14Z

Codecov Report

❌ Patch coverage is 64.42953% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.17%. Comparing base (ae4ea94) to head (111658e).

Files with missing lines	Patch %	Lines
petab/v2/core.py	61.90%	32 Missing and 8 partials ⚠️
petab/v2/lint.py	69.76%	6 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #482      +/-   ##
==========================================
- Coverage   75.29%   75.17%   -0.12%     
==========================================
  Files          62       62              
  Lines        6946     7086     +140     
  Branches     1229     1254      +25     
==========================================
+ Hits         5230     5327      +97     
- Misses       1245     1277      +32     
- Partials      471      482      +11

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dilpath

Thanks! Overall it would be good to try to separate as much of the SciML things into separate files or subdirectory if possible. This will help later on when we design a nicer extension interface for libpetab-python, I think. It will also help with maintenance immediately.

dilpath · 2026-06-16T11:22:28Z

    #: Nominal value.
    nominal_value: Annotated[
-        float | None, BeforeValidator(_convert_nan_to_none)
+        float | Literal["array"] | None, BeforeValidator(_convert_nan_to_none)


Maybe add some # PEtab SciML supports arrays comment here, otherwise a non-SciML user/dev might wonder why "array" is permitted

dilpath · 2026-06-16T12:19:50Z

        return sum(p.estimate for p in self.parameters)


+class Hybridization(BaseModel):


Move SciML-specific things to a separate file or subdirectory e.g. sciml/core.py and import them here? Just for organization; might help to keep SciML things as separate as possible. e.g. do not import petab_sciml here, rather only in the separated file.

Nevermind this if it will lead to circular imports etc.

dilpath · 2026-06-16T12:24:14Z

        config: ProblemConfig = None,
    ):
-        from ..v2.lint import default_validation_tasks
+        from ..v2.lint import default_validation_tasks, sciml_validation_tasks


Also this, import from ..v2.sciml.lint for example, for separation.

dilpath · 2026-06-16T12:29:33Z

+    def add_hybridization(self, target_id: str, target_value: str):
+        """Add a SciML hybridization table entry to the problem.
+
+        If there are more than one hybridization tables, the hybridization


Suggested change

If there are more than one hybridization tables, the hybridization

If there is more than one hybridization table, the hybridization

dilpath · 2026-06-16T12:29:45Z

+        """Add a SciML hybridization table entry to the problem.
+
+        If there are more than one hybridization tables, the hybridization
+        is added to the last one.


Suggested change

is added to the last one.

is added to the last table.

dilpath · 2026-06-16T12:37:08Z

+            for mapping in problem.mappings:
+                # petabEntityId is not referenced in any model
+                for model in problem.models:
+                    if 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}`."
+                        )


The spec currently says that the petabEntityId can be the same as a modelEntityId, hence the petabEntityIds can appear in the model, right? @dweindl what do you think?

The petabEntityId may be the same as the modelEntityId, but it must not be used to alias an entity that already has a valid PEtab identifier. This restriction is to avoid unnecessary complexity in the PEtab problem files.

There is some discussion of this here PEtab-dev/PEtab#669

dilpath · 2026-06-16T12:38:25Z

+        if culprits := (hyb_target_vals & nn_input_ids):
+            messages.append(
+                "The following neural net inputs were used as target values "
+                f"in the Hybridization tbale: `{culprits}`. Please simplify."


Suggested change

f"in the Hybridization tbale: `{culprits}`. Please simplify."

f"in the Hybridization table: `{culprits}`."

dilpath · 2026-06-16T12:42:37Z

+    # Add petab ids from mapping table if they are used for aliasing
    for mapping in problem.mappings:
-        if mapping.model_id and mapping.model_id in parameter_ids.keys():
+        if mapping.petab_id not in invalid:
            parameter_ids[mapping.petab_id] = None
+        # An aliased model id is not a valid parameter id
+        if mapping.model_id and mapping.model_id in parameter_ids:
+            del parameter_ids[mapping.model_id]


As above, I think it's OK to map a model ID to a PEtab ID in the mapping table as long as it's not an alias, i.e. only in the special case where petabEntityId == modelEntityId. I think the reason for this in the spec is for support use of the mapping table for annotation, i.e. via additional user-defined annotation columns.

m-philipps changed the title ~~Support a PEtab SciML problem and enable linting~~ Support PEtab SciML problems and enable linting May 26, 2026

m-philipps and others added 20 commits June 11, 2026 15:38

update data types

3d552a9

add checks for mapping table

d003839

Use SBML in test

c3e8819

ensure that all observables have a noise_formula

f4278c9

rollback

0a54558

add hybridization class

db6fbd6

add sciml problem config

5f89cab

create Problem for sciml extension

2fbc20e

Assign SciML-specific lint checks

1c4330c

remove lint checks that are covered by pydantic

648a21a

Proble.add_hybridization method

be625d6

add hybridization df property, setter

32c4b4b

fix

0de7a5f

add a basic test for sciml

5499ee1

methods for adding nn, array data to problem

64b8c25

Non-local petab_sciml import

e426cb7

add dependency

8227ce0

Update petab_sciml dependency to use Git URL

0123281

update allowed params logic for mappings

317b810

neural_nets to neural_networks

cb5cef1

BSnelling force-pushed the sciml_lint branch from 3295eec to cb5cef1 Compare June 11, 2026 14:57

BSnelling added 2 commits June 15, 2026 10:38

add required params check to sciml validations

4a208f9

ruff format

111658e

BSnelling marked this pull request as ready for review June 16, 2026 10:41

BSnelling requested a review from a team as a code owner June 16, 2026 10:41

dilpath requested changes Jun 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support PEtab SciML problems and enable linting#482

Support PEtab SciML problems and enable linting#482
m-philipps wants to merge 22 commits into
mainfrom
sciml_lint

m-philipps commented May 26, 2026

Uh oh!

codecov-commenter commented Jun 11, 2026 •

edited

Loading

Uh oh!

dilpath left a comment

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

dilpath Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return sum(p.estimate for p in self.parameters)


		class Hybridization(BaseModel):

	If there are more than one hybridization tables, the hybridization
	If there is more than one hybridization table, the hybridization

	f"in the Hybridization tbale: `{culprits}`. Please simplify."
	f"in the Hybridization table: `{culprits}`."

Conversation

m-philipps commented May 26, 2026

Uh oh!

codecov-commenter commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dilpath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jun 11, 2026 •

edited

Loading