Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions doc/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Jump to:

Description

- Remove pydantic dependency from MLI code
- Update MLI environment variables using new naming convention
- Reduce a copy by using torch.from_numpy instead of torch.tensor
- Enable dynamic feature store selection
Expand Down
20 changes: 15 additions & 5 deletions smartsim/_core/mli/infrastructure/storage/featurestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,32 @@

import typing as t
from abc import ABC, abstractmethod

from pydantic import BaseModel, Field
from dataclasses import dataclass

from smartsim.log import get_logger

logger = get_logger(__name__)


class FeatureStoreKey(BaseModel):
@dataclass(frozen=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want frozen=True? I'm not familiar with the BaseModel -- is that frozen by default?

If frozen is not used, then I would assume that we need to add setter methods to check that any updates adhere to non-empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BaseModels are not frozen by default, but I think we definitely want a frozen dataclass because we don't want FeatureStoreKey instances to be updated.

class FeatureStoreKey:
"""A key,descriptor pair enabling retrieval of an item from a feature store"""

key: str = Field(min_length=1)
key: str
"""The unique key of an item in a feature store"""
descriptor: str = Field(min_length=1)
descriptor: str
"""The unique identifier of the feature store containing the key"""

def __post_init__(self) -> None:
"""Ensure the key and descriptor have at least one character
:raises ValueError: if key or descriptor are empty strings
"""
if len(self.key) < 1:
raise ValueError("Key must have at least one character.")
if len(self.descriptor) < 1:
raise ValueError("Descriptor must have at least one character.")


class FeatureStore(ABC):
"""Abstract base class providing the common interface for retrieving
Expand Down
12 changes: 12 additions & 0 deletions tests/mli/test_core_machine_learning_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,15 @@ def test_place_outputs() -> None:

for i in range(3):
assert feature_store[keys[i].key] == data[i]


@pytest.mark.parametrize(
"key, descriptor",
[
pytest.param("", "desc", id="invalid key"),
pytest.param("key", "", id="invalid descriptor"),
],
)
def test_invalid_featurestorekey(key, descriptor) -> None:
with pytest.raises(ValueError):
fsk = FeatureStoreKey(key, descriptor)