From 4d088103b2801c90e3d6bea5925ea9c4a5455509 Mon Sep 17 00:00:00 2001 From: Sebastian Echeverria Date: Mon, 25 Sep 2023 18:22:27 -0400 Subject: [PATCH 1/2] Modified Property so it can load properties from any module, not only the mlte.property package. This allows external extension of properties outside of MLTE. --- demo/requirements.ipynb | 10 ++--- mlte/property/__init__.py | 3 -- mlte/property/costs/__init__.py | 5 --- mlte/property/costs/storage_cost.py | 5 ++- mlte/property/costs/training_compute_cost.py | 5 ++- mlte/property/costs/training_memory_cost.py | 5 ++- mlte/property/functionality/__init__.py | 3 -- mlte/property/functionality/task_efficacy.py | 3 +- mlte/property/property.py | 44 +++++++++----------- mlte/spec/model.py | 3 ++ mlte/spec/spec.py | 2 +- mlte/validation/validated_spec.py | 1 + test/property/test_properties.py | 10 ++--- test/schema/test_spec_schema.py | 2 +- test/schema/test_validatedspec_schema.py | 2 +- test/spec/extended_property.py | 27 ++++++++++++ test/spec/test_model.py | 1 + test/spec/test_spec.py | 21 +++++++++- test/validation/test_model.py | 1 + test/validation/test_specvalidator.py | 2 +- test/validation/test_validatedspec.py | 2 +- 21 files changed, 96 insertions(+), 61 deletions(-) create mode 100644 test/spec/extended_property.py diff --git a/demo/requirements.ipynb b/demo/requirements.ipynb index 6fd92901a..52df81fcd 100644 --- a/demo/requirements.ipynb +++ b/demo/requirements.ipynb @@ -54,12 +54,10 @@ "source": [ "from mlte.spec.spec import Spec\n", "\n", - "from mlte.property.costs import (\n", - " StorageCost,\n", - " TrainingComputeCost,\n", - " TrainingMemoryCost\n", - ")\n", - "from mlte.property.functionality import TaskEfficacy\n", + "from mlte.property.costs.storage_cost import StorageCost\n", + "from mlte.property.costs.training_memory_cost import TrainingMemoryCost\n", + "from mlte.property.costs.training_compute_cost import TrainingComputeCost\n", + "from mlte.property.functionality.task_efficacy import TaskEfficacy\n", "\n", "\n", "from mlte.measurement.storage import LocalObjectSize\n", diff --git a/mlte/property/__init__.py b/mlte/property/__init__.py index 3f4cc8136..e69de29bb 100644 --- a/mlte/property/__init__.py +++ b/mlte/property/__init__.py @@ -1,3 +0,0 @@ -from .property import Property - -__all__ = ["Property"] diff --git a/mlte/property/costs/__init__.py b/mlte/property/costs/__init__.py index afedbd95f..e69de29bb 100644 --- a/mlte/property/costs/__init__.py +++ b/mlte/property/costs/__init__.py @@ -1,5 +0,0 @@ -from .storage_cost import StorageCost -from .training_compute_cost import TrainingComputeCost -from .training_memory_cost import TrainingMemoryCost - -__all__ = ["StorageCost", "TrainingMemoryCost", "TrainingComputeCost"] diff --git a/mlte/property/costs/storage_cost.py b/mlte/property/costs/storage_cost.py index fbf1ce108..dc1597cbd 100644 --- a/mlte/property/costs/storage_cost.py +++ b/mlte/property/costs/storage_cost.py @@ -4,8 +4,8 @@ StorageCost property definition. """ -from ..._private.text import cleantext -from ..property import Property +from mlte._private.text import cleantext +from mlte.property.property import Property class StorageCost(Property): @@ -26,4 +26,5 @@ def __init__(self, rationale: str): """ ), rationale, + __name__, ) diff --git a/mlte/property/costs/training_compute_cost.py b/mlte/property/costs/training_compute_cost.py index 2db12f572..f7d1a82fa 100644 --- a/mlte/property/costs/training_compute_cost.py +++ b/mlte/property/costs/training_compute_cost.py @@ -4,8 +4,8 @@ TrainingComputeCost property definition. """ -from ..._private.text import cleantext -from ..property import Property +from mlte._private.text import cleantext +from mlte.property.property import Property class TrainingComputeCost(Property): @@ -31,4 +31,5 @@ def __init__(self, rationale: str): """ ), rationale, + __name__, ) diff --git a/mlte/property/costs/training_memory_cost.py b/mlte/property/costs/training_memory_cost.py index 3f644d8a9..ec54d55f0 100644 --- a/mlte/property/costs/training_memory_cost.py +++ b/mlte/property/costs/training_memory_cost.py @@ -4,8 +4,8 @@ TrainingMemoryCost property definition. """ -from ..._private.text import cleantext -from ..property import Property +from mlte._private.text import cleantext +from mlte.property.property import Property class TrainingMemoryCost(Property): @@ -31,4 +31,5 @@ def __init__(self, rationale: str): """ ), rationale, + __name__, ) diff --git a/mlte/property/functionality/__init__.py b/mlte/property/functionality/__init__.py index f2493ade5..e69de29bb 100644 --- a/mlte/property/functionality/__init__.py +++ b/mlte/property/functionality/__init__.py @@ -1,3 +0,0 @@ -from .task_efficacy import TaskEfficacy - -__all__ = ["TaskEfficacy"] diff --git a/mlte/property/functionality/task_efficacy.py b/mlte/property/functionality/task_efficacy.py index c66a50f80..51e04ac71 100644 --- a/mlte/property/functionality/task_efficacy.py +++ b/mlte/property/functionality/task_efficacy.py @@ -5,7 +5,7 @@ """ from mlte._private.text import cleantext -from mlte.property import Property +from mlte.property.property import Property class TaskEfficacy(Property): @@ -28,4 +28,5 @@ def __init__(self, rationale: str): """ ), rationale, + __name__, ) diff --git a/mlte/property/property.py b/mlte/property/property.py index ead7172aa..81433b648 100644 --- a/mlte/property/property.py +++ b/mlte/property/property.py @@ -8,7 +8,6 @@ import abc import importlib -import pkgutil from typing import Type import mlte._private.meta as meta @@ -23,16 +22,15 @@ def __subclasshook__(cls, subclass): """Define the interface for all concrete properties.""" return meta.has_callables(subclass, "__init__") - def __init__(self, name: str, description: str, rationale: str): + def __init__( + self, name: str, description: str, rationale: str, module: str + ): """ Initialize a Property instance. :param name: The name of the property - :type name: str :param description: The description of the property - :type description: str :param rationale: The rationale for using the property - :type rationale: str """ self.name: str = name """The name of the property.""" @@ -43,17 +41,20 @@ def __init__(self, name: str, description: str, rationale: str): self.rationale: str = rationale """The rationale for using the property.""" + self.module: str = module + """The name of the module the property is defined in.""" + def to_model(self) -> PropertyModel: """ Return a Property as a model. :return: The property as its model. - :rtype: PropertyModel """ return PropertyModel( name=self.name, description=self.description, rationale=self.rationale, + module=self.module, ) @classmethod @@ -62,10 +63,8 @@ def from_model(cls, model: PropertyModel) -> Property: Load a Property instance from a model. :param model: The model with the Property info. - :type model: PropertyModel :return: The loaded property - :rtype: Property """ if model.name == "": raise RuntimeError( @@ -74,22 +73,17 @@ def from_model(cls, model: PropertyModel) -> Property: classname = model.name # Load the class type from the module - properties_package_name = "mlte.property" - properties_module = importlib.import_module( - properties_package_name, package="mlte" - ) - for submodule_info in pkgutil.iter_modules( - properties_module.__path__, properties_module.__name__ + "." - ): - submodule = importlib.import_module( - submodule_info.name, package=properties_package_name + module_path = model.module + try: + properties_module = importlib.import_module(module_path) + except Exception: + raise RuntimeError(f"Module {module_path} not found") + try: + class_: Type[Property] = getattr(properties_module, classname) + except AttributeError: + raise RuntimeError( + f"Property {model.name} in module {module_path} not found" ) - try: - class_: Type[Property] = getattr(submodule, classname) - except AttributeError: - continue - - # Instantiate the property - return class_(model.rationale) # type: ignore - raise RuntimeError(f"Property {model.name} not found") + # Instantiate the property + return class_(model.rationale) # type: ignore diff --git a/mlte/spec/model.py b/mlte/spec/model.py index 425681a5d..a8b89e227 100644 --- a/mlte/spec/model.py +++ b/mlte/spec/model.py @@ -38,6 +38,9 @@ class PropertyModel(BaseModel): conditions: Dict[str, ConditionModel] = {} """A dictionary of conditions, keyed by measurement id, to be validated for this property.""" + module: str + """The full package and module path of the Property class.""" + class SpecModel(BaseModel): """The model implementation for the Spec artifact.""" diff --git a/mlte/spec/spec.py b/mlte/spec/spec.py index fc69c38e5..5928915aa 100644 --- a/mlte/spec/spec.py +++ b/mlte/spec/spec.py @@ -12,7 +12,7 @@ from mlte.artifact.artifact import Artifact from mlte.artifact.model import ArtifactModel from mlte.artifact.type import ArtifactType -from mlte.property import Property +from mlte.property.property import Property from mlte.spec.condition import Condition from mlte.spec.model import PropertyModel, SpecModel diff --git a/mlte/validation/validated_spec.py b/mlte/validation/validated_spec.py index baec56cf0..c4c4ab112 100644 --- a/mlte/validation/validated_spec.py +++ b/mlte/validation/validated_spec.py @@ -93,6 +93,7 @@ def to_model(self) -> ArtifactModel: description=property_model.description, rationale=property_model.rationale, conditions=property_model.conditions, + module=property_model.module, results=res_model[property_model.name], ) for property_model in spec_model.properties diff --git a/test/property/test_properties.py b/test/property/test_properties.py index 26e46c249..ed2c44e36 100644 --- a/test/property/test_properties.py +++ b/test/property/test_properties.py @@ -4,12 +4,10 @@ Unit tests for model properties. """ -from mlte.property.costs import ( - StorageCost, - TrainingComputeCost, - TrainingMemoryCost, -) -from mlte.property.functionality import TaskEfficacy +from mlte.property.costs.storage_cost import StorageCost +from mlte.property.costs.training_compute_cost import TrainingComputeCost +from mlte.property.costs.training_memory_cost import TrainingMemoryCost +from mlte.property.functionality.task_efficacy import TaskEfficacy def test_storage_cost(): diff --git a/test/schema/test_spec_schema.py b/test/schema/test_spec_schema.py index e045c9945..c296af481 100644 --- a/test/schema/test_spec_schema.py +++ b/test/schema/test_spec_schema.py @@ -4,7 +4,7 @@ Unit tests for Spec schema. """ -from mlte.property.costs import StorageCost +from mlte.property.costs.storage_cost import StorageCost from mlte.spec.spec import Spec from mlte.value.types.integer import Integer diff --git a/test/schema/test_validatedspec_schema.py b/test/schema/test_validatedspec_schema.py index 88f1e837b..38921568c 100644 --- a/test/schema/test_validatedspec_schema.py +++ b/test/schema/test_validatedspec_schema.py @@ -6,7 +6,7 @@ from mlte.evidence.metadata import EvidenceMetadata, Identifier -from mlte.property.costs import StorageCost +from mlte.property.costs.storage_cost import StorageCost from mlte.spec.spec import Spec from mlte.validation.spec_validator import SpecValidator from mlte.value.types.integer import Integer diff --git a/test/spec/extended_property.py b/test/spec/extended_property.py new file mode 100644 index 000000000..0a58e46a9 --- /dev/null +++ b/test/spec/extended_property.py @@ -0,0 +1,27 @@ +""" +test/spec/test_model.py + +ExtendedProperty definition, for testing purposes. +""" + +from mlte._private.text import cleantext +from mlte.property.property import Property + + +class ExtendedProperty(Property): + """ + The ExtendedProperty property is a property not defined in the default property package. + """ + + def __init__(self, rationale: str): + """Initialize a ExtendedProperty instance.""" + super().__init__( + self.__class__.__name__, + cleantext( + """ + The ExtendedProperty property is just for testing purposes. + """ + ), + rationale, + __name__, + ) diff --git a/test/spec/test_model.py b/test/spec/test_model.py index 90e5932ec..ae1506d95 100644 --- a/test/spec/test_model.py +++ b/test/spec/test_model.py @@ -24,6 +24,7 @@ def test_spec_body() -> None: name="TaskEfficacy", description="Property for useful things.", rationale="Because I say so", + module="mlte.properties.functionality.task_efficacy", conditions={ "accuracy": model.ConditionModel( name="less_than", diff --git a/test/spec/test_spec.py b/test/spec/test_spec.py index 26675f4ae..5f7da1d67 100644 --- a/test/spec/test_spec.py +++ b/test/spec/test_spec.py @@ -6,13 +6,14 @@ from __future__ import annotations +from test.spec.extended_property import ExtendedProperty from typing import Tuple import pytest from mlte.context.context import Context from mlte.measurement.storage import LocalObjectSize -from mlte.property.costs import StorageCost +from mlte.property.costs.storage_cost import StorageCost from mlte.spec.spec import Spec from mlte.store.base import Store @@ -66,3 +67,21 @@ def test_non_unique_properties(): StorageCost("rationale2"): {}, }, ) + + +def test_save_load_extended_property( + store_with_context: Tuple[Store, Context] # noqa +): + store, ctx = store_with_context + + s = Spec( + properties={ + ExtendedProperty("rationale"): { + "test": LocalObjectSize.value().less_than(3) + } + }, + ) + s.save_with(ctx, store) + + loaded = Spec.load_with(context=ctx, store=store) + assert s == loaded diff --git a/test/validation/test_model.py b/test/validation/test_model.py index 679be4681..848e9501b 100644 --- a/test/validation/test_model.py +++ b/test/validation/test_model.py @@ -26,6 +26,7 @@ def test_validated_spec_body() -> None: name="TaskEfficacy", description="Property for useful things.", rationale="Because I say so", + module="mlte.properties.functionality.task_efficacy", conditions={ "accuracy": ConditionModel( name="less_than", diff --git a/test/validation/test_specvalidator.py b/test/validation/test_specvalidator.py index 59e02ec78..837c2a68a 100644 --- a/test/validation/test_specvalidator.py +++ b/test/validation/test_specvalidator.py @@ -8,7 +8,7 @@ import pytest from mlte.evidence.metadata import EvidenceMetadata, Identifier -from mlte.property.costs import StorageCost +from mlte.property.costs.storage_cost import StorageCost from mlte.spec.spec import Spec from mlte.validation.spec_validator import SpecValidator from mlte.value.types.integer import Integer diff --git a/test/validation/test_validatedspec.py b/test/validation/test_validatedspec.py index cc2e93358..ae241b0c7 100644 --- a/test/validation/test_validatedspec.py +++ b/test/validation/test_validatedspec.py @@ -12,7 +12,7 @@ from mlte.context.context import Context from mlte.evidence.metadata import EvidenceMetadata, Identifier -from mlte.property.costs import StorageCost +from mlte.property.costs.storage_cost import StorageCost from mlte.spec.spec import Spec from mlte.store.base import Store from mlte.validation.result import Result From 17b5a3b40052252502587c53dc4999781443bb56 Mon Sep 17 00:00:00 2001 From: Sebastian Echeverria Date: Mon, 25 Sep 2023 20:08:10 -0400 Subject: [PATCH 2/2] Updating schema --- .../assets/schema/artifact/spec/v0.0.1/schema.json | 7 ++++++- .../assets/schema/artifact/validated/v0.0.1/schema.json | 7 ++++++- mlte/schema/artifact/spec/v0.0.1/schema.json | 7 ++++++- mlte/schema/artifact/validated/v0.0.1/schema.json | 7 ++++++- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/mlte/frontend/nuxt-app/assets/schema/artifact/spec/v0.0.1/schema.json b/mlte/frontend/nuxt-app/assets/schema/artifact/spec/v0.0.1/schema.json index 825336ec2..897e90268 100644 --- a/mlte/frontend/nuxt-app/assets/schema/artifact/spec/v0.0.1/schema.json +++ b/mlte/frontend/nuxt-app/assets/schema/artifact/spec/v0.0.1/schema.json @@ -63,10 +63,15 @@ "default": {}, "title": "Conditions", "type": "object" + }, + "module": { + "title": "Module", + "type": "string" } }, "required": [ - "name" + "name", + "module" ], "title": "PropertyModel", "type": "object" diff --git a/mlte/frontend/nuxt-app/assets/schema/artifact/validated/v0.0.1/schema.json b/mlte/frontend/nuxt-app/assets/schema/artifact/validated/v0.0.1/schema.json index 1dde5890b..fe99de26e 100644 --- a/mlte/frontend/nuxt-app/assets/schema/artifact/validated/v0.0.1/schema.json +++ b/mlte/frontend/nuxt-app/assets/schema/artifact/validated/v0.0.1/schema.json @@ -108,6 +108,10 @@ "title": "Conditions", "type": "object" }, + "module": { + "title": "Module", + "type": "string" + }, "results": { "additionalProperties": { "$ref": "#/$defs/ResultModel" @@ -118,7 +122,8 @@ } }, "required": [ - "name" + "name", + "module" ], "title": "PropertyAndResultsModel", "type": "object" diff --git a/mlte/schema/artifact/spec/v0.0.1/schema.json b/mlte/schema/artifact/spec/v0.0.1/schema.json index 825336ec2..897e90268 100644 --- a/mlte/schema/artifact/spec/v0.0.1/schema.json +++ b/mlte/schema/artifact/spec/v0.0.1/schema.json @@ -63,10 +63,15 @@ "default": {}, "title": "Conditions", "type": "object" + }, + "module": { + "title": "Module", + "type": "string" } }, "required": [ - "name" + "name", + "module" ], "title": "PropertyModel", "type": "object" diff --git a/mlte/schema/artifact/validated/v0.0.1/schema.json b/mlte/schema/artifact/validated/v0.0.1/schema.json index 1dde5890b..fe99de26e 100644 --- a/mlte/schema/artifact/validated/v0.0.1/schema.json +++ b/mlte/schema/artifact/validated/v0.0.1/schema.json @@ -108,6 +108,10 @@ "title": "Conditions", "type": "object" }, + "module": { + "title": "Module", + "type": "string" + }, "results": { "additionalProperties": { "$ref": "#/$defs/ResultModel" @@ -118,7 +122,8 @@ } }, "required": [ - "name" + "name", + "module" ], "title": "PropertyAndResultsModel", "type": "object"