-
Notifications
You must be signed in to change notification settings - Fork 9
Support PEtab SciML problems and enable linting #482
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
Open
m-philipps
wants to merge
28
commits into
lint_mapping_table
Choose a base branch
from
sciml_lint
base: lint_mapping_table
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
1eb9dec
add hybridization class
m-philipps 1165218
add sciml problem config
m-philipps f2c4193
create Problem for sciml extension
m-philipps 96858f2
Assign SciML-specific lint checks
m-philipps dd3f87b
Proble.add_hybridization method
m-philipps 8c1a8a9
add hybridization df property, setter
m-philipps 8c5cd2b
fix
m-philipps 238829b
add a basic test for sciml
m-philipps d4d8a94
methods for adding nn, array data to problem
m-philipps 653ccc7
Non-local petab_sciml import
m-philipps 15c4486
add dependency
m-philipps 3d8901b
Update petab_sciml dependency to use Git URL
dilpath 4ef0a10
neural_nets to neural_networks
BSnelling 58c549a
add required params check to sciml validations
BSnelling f1d4540
ruff format
BSnelling 2f76d3e
implement feedback from code review
BSnelling 81292a7
move sciml code to separate files and resolve circular imports
BSnelling 840ae7e
fixup ruff
BSnelling 9af9a54
fix docs build issue
BSnelling e342f26
Update petab/v2/core.py
BSnelling 254a596
add to docstrings
BSnelling d95f09a
Revert "move sciml code to separate files and resolve circular imports"
BSnelling 239e1ff
use problem.extensions.sciml for separation
BSnelling 3c1a09f
revert mapping table fixes that should be implemented in the other pr
dilpath 118aed7
fix aliased model logic - again
BSnelling e133f21
move sciml validation to own file
BSnelling 81148e6
use pypi petab_sciml
BSnelling 7759b50
cherry-pick ci fix
BSnelling File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,6 +309,22 @@ def __iadd__(self, other: T) -> BaseTable[T]: | |
| return self | ||
|
|
||
|
|
||
| # SciML extension classes — imported after BaseTable is defined to avoid | ||
| # circular imports (sciml.py does not import from core.py). | ||
| from .extensions.sciml import ( # noqa: E402 | ||
| HybridizationTable, | ||
| SciMLConfig, | ||
| SciMLExt, | ||
| ) | ||
|
|
||
|
|
||
| class ProblemExtensions: | ||
| """Runtime extension state attached to a :class:`Problem`.""" | ||
|
|
||
| def __init__(self, sciml: SciMLExt = None): | ||
| self.sciml: SciMLExt = sciml or SciMLExt() | ||
|
|
||
|
|
||
| class Observable(BaseModel): | ||
| """Observable definition.""" | ||
|
|
||
|
|
@@ -927,6 +943,7 @@ class Parameter(BaseModel): | |
| ) | ||
| #: Nominal value. | ||
| nominal_value: Annotated[ | ||
| # PEtab SciML supports arrays via "array" nominal values | ||
| float | Literal["array"] | None, BeforeValidator(_convert_nan_to_none) | ||
| ] = Field(alias=C.NOMINAL_VALUE, default=None) | ||
| #: Is the parameter to be estimated? | ||
|
|
@@ -1134,22 +1151,33 @@ def __init__( | |
| measurement_tables: list[MeasurementTable] = None, | ||
| parameter_tables: list[ParameterTable] = None, | ||
| mapping_tables: list[MappingTable] = None, | ||
| extensions: ProblemExtensions = None, | ||
| config: ProblemConfig = None, | ||
| ): | ||
| from ..v2.lint import default_validation_tasks | ||
| from ..v2.lint import default_validation_tasks, sciml_validation_tasks | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this, import |
||
|
|
||
| self.config = config | ||
| self.models: list[Model] = models or [] | ||
| self.validation_tasks: list[ValidationTask] = ( | ||
| default_validation_tasks.copy() | ||
| ) | ||
| if ( | ||
| config | ||
| and config.extensions | ||
| and config.extensions.get(C.EXT_ID_SCIML) | ||
| ): | ||
| self.validation_tasks: list[ValidationTask] = ( | ||
| sciml_validation_tasks.copy() | ||
| ) | ||
| else: | ||
| self.validation_tasks: list[ValidationTask] = ( | ||
| default_validation_tasks.copy() | ||
| ) | ||
|
|
||
| self.observable_tables = observable_tables or [ObservableTable()] | ||
| self.condition_tables = condition_tables or [ConditionTable()] | ||
| self.experiment_tables = experiment_tables or [ExperimentTable()] | ||
| self.measurement_tables = measurement_tables or [MeasurementTable()] | ||
| self.mapping_tables = mapping_tables or [MappingTable()] | ||
| self.parameter_tables = parameter_tables or [ParameterTable()] | ||
| self.extensions = extensions or ProblemExtensions() | ||
|
|
||
| def __repr__(self): | ||
| return f"<{self.__class__.__name__} id={self.id!r}>" | ||
|
|
@@ -1322,6 +1350,45 @@ def from_yaml( | |
| else None | ||
| ) | ||
|
|
||
| extensions = ProblemExtensions() | ||
| if config.extensions and config.extensions.get(C.EXT_ID_SCIML): | ||
| from petab_sciml import ArrayDataStandard, NNModel, NNModelStandard | ||
|
|
||
| # Neural network classes are constructed via pytorch for now to get | ||
| # the proper inputs | ||
| neural_networks = [ | ||
| NNModel.from_pytorch_module( | ||
| NNModelStandard.load_data( | ||
| _generate_path( | ||
| file_path=nn_config.location, | ||
| base_path=base_path, | ||
| ) | ||
| ).to_pytorch_module(), | ||
| nn_model_id=nn_id, | ||
| ) | ||
| for nn_id, nn_config in ( | ||
| config.extensions[C.EXT_ID_SCIML].neural_networks or {} | ||
| ).items() | ||
| ] | ||
|
|
||
| hybridization_tables = [ | ||
| HybridizationTable.from_tsv(f, base_path) | ||
| for f in config.extensions[C.EXT_ID_SCIML].hybridization_files | ||
| ] | ||
|
|
||
| array_data_files = [ | ||
| ArrayDataStandard.load_data(_generate_path(f, base_path)) | ||
| for f in config.extensions[C.EXT_ID_SCIML].array_files | ||
| ] | ||
|
|
||
| extensions = ProblemExtensions( | ||
| sciml=SciMLExt( | ||
| neural_networks=neural_networks, | ||
| hybridization_tables=hybridization_tables, | ||
| array_data_files=array_data_files, | ||
| ) | ||
| ) | ||
|
|
||
| return Problem( | ||
| config=config, | ||
| models=models, | ||
|
|
@@ -1331,6 +1398,7 @@ def from_yaml( | |
| measurement_tables=measurement_tables, | ||
| parameter_tables=parameter_tables, | ||
| mapping_tables=mapping_tables, | ||
| extensions=extensions, | ||
| ) | ||
|
|
||
| @staticmethod | ||
|
|
@@ -1941,14 +2009,21 @@ def validate( | |
|
|
||
| validation_results = ValidationResultList() | ||
|
|
||
| if self.config and self.config.extensions: | ||
| extensions = ",".join(self.config.extensions.keys()) | ||
| supported_extensions = {C.EXT_ID_SCIML} | ||
| if ( | ||
| self.config | ||
| and self.config.extensions | ||
| and (self.config.extensions.keys() - supported_extensions) | ||
| ): | ||
| extensions_without_support = ",".join( | ||
| self.config.extensions.keys() - supported_extensions | ||
| ) | ||
| validation_results.append( | ||
| ValidationIssue( | ||
| ValidationIssueSeverity.WARNING, | ||
| "Validation of PEtab extensions is not yet implemented, " | ||
| "but the given problem uses the following extensions: " | ||
| f"{extensions}", | ||
| "The given problem uses the following extensions for " | ||
| "which validation is not yet implemented: " | ||
| f"{extensions_without_support}", | ||
| ) | ||
| ) | ||
|
|
||
|
|
@@ -2506,6 +2581,23 @@ class ProblemConfig(BaseModel): | |
| validate_assignment=True, | ||
| ) | ||
|
|
||
| @field_validator("extensions", mode="before") | ||
| @classmethod | ||
| def _parse_extensions(cls, v): | ||
| """Parse extensions dict and convert known extensions to their specific | ||
| config classes.""" | ||
| if isinstance(v, dict): | ||
| parsed_extensions = {} | ||
| for ext_name, ext_config in v.items(): | ||
| if ext_name == C.EXT_ID_SCIML: | ||
| # Convert sciml extension to SciMLConfig | ||
| parsed_extensions[ext_name] = SciMLConfig(**ext_config) | ||
| else: | ||
| # Keep other extensions as ExtensionConfig | ||
| parsed_extensions[ext_name] = ExtensionConfig(**ext_config) | ||
| return parsed_extensions | ||
| return v | ||
|
|
||
| # convert parameter_file to list | ||
| @field_validator( | ||
| "parameter_files", | ||
|
|
@@ -2543,12 +2635,22 @@ def to_yaml(self, filename: str | Path): | |
|
|
||
| for model_id in data.get("model_files", {}): | ||
| data["model_files"][model_id][C.MODEL_LOCATION] = str( | ||
| data["model_files"][model_id]["location"] | ||
| data["model_files"][model_id][C.MODEL_LOCATION] | ||
| ) | ||
| if data["id"] is None: | ||
| # The schema requires a valid id or no id field at all. | ||
| del data["id"] | ||
|
|
||
| for ext_id, d_ext in data[C.EXTENSIONS].items(): | ||
| if ext_id == C.EXT_ID_SCIML: | ||
| # convert Paths to strings | ||
| for key in ("array_files", "hybridization_files"): | ||
| d_ext[key] = list(map(str, d_ext[key])) | ||
| for nn in d_ext["neural_networks"]: | ||
| d_ext["neural_networks"][nn][C.MODEL_LOCATION] = str( | ||
| d_ext["neural_networks"][nn][C.MODEL_LOCATION] | ||
| ) | ||
|
|
||
| write_yaml(data, filename) | ||
|
|
||
| @property | ||
|
|
||
Empty file.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.