Skip to content

Commit

Permalink
[ENH] fix HNSW param defaults in new configuration logic
Browse files Browse the repository at this point in the history
  • Loading branch information
codetheweb committed Jul 16, 2024
1 parent 18e0a6b commit 81cb3cf
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
35 changes: 31 additions & 4 deletions chromadb/api/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ def __init__(self, parameters: Optional[List[ConfigurationParameter]] = None):
name=name, value=definition.default_value
)

(is_valid, error_msg) = self.validator()
if not is_valid:
if error_msg:
raise ValueError(f"Invalid configuration: {error_msg}")
raise ValueError("Invalid configuration")

def __repr__(self) -> str:
return f"Configuration({self.parameter_map.values()})"

Expand All @@ -129,6 +135,13 @@ def __eq__(self, __value: object) -> bool:
return NotImplemented
return self.parameter_map == __value.parameter_map

def validator(self) -> tuple[bool, Optional[str]]:
"""Perform custom validation when parameters are dependent on each other.
Returns a tuple with a boolean indicating whether the configuration is valid and an optional error message.
"""
return (True, None)

def get_parameters(self) -> List[ConfigurationParameter]:
"""Returns the parameters of the configuration."""
return list(self.parameter_map.values())
Expand Down Expand Up @@ -247,16 +260,30 @@ class HNSWConfigurationInternal(ConfigurationInternal):
name="batch_size",
validator=lambda value: isinstance(value, int) and value >= 1,
is_static=True,
default_value=1000,
default_value=100,
),
"sync_threshold": ConfigurationDefinition(
name="sync_threshold",
validator=lambda value: isinstance(value, int) and value >= 1,
is_static=True,
default_value=100,
default_value=1000,
),
}

@override
def validator(self) -> tuple[bool, Optional[str]]:
batch_size = self.parameter_map.get("batch_size")
sync_threshold = self.parameter_map.get("sync_threshold")

if (
batch_size
and sync_threshold
and cast(int, batch_size.value) > cast(int, sync_threshold.value)
):
return (False, "batch_size must be less than or equal to sync_threshold")

return super().validator()

@classmethod
def from_legacy_params(cls, params: Dict[str, Any]) -> Self:
"""Returns an HNSWConfiguration from a metadata dict containing legacy HNSW parameters. Used for migration."""
Expand Down Expand Up @@ -302,8 +329,8 @@ def __init__(
num_threads: int = cpu_count(),
M: int = 16,
resize_factor: float = 1.2,
batch_size: int = 1000,
sync_threshold: int = 100,
batch_size: int = 100,
sync_threshold: int = 1000,
):
parameters = [
ConfigurationParameter(name="space", value=space),
Expand Down
6 changes: 6 additions & 0 deletions chromadb/test/configurations/test_configurations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
ConfigurationDefinition,
StaticParameterError,
ConfigurationParameter,
HNSWConfiguration,
)


Expand Down Expand Up @@ -76,3 +77,8 @@ def test_validation() -> None:
]
with pytest.raises(ValueError):
TestConfiguration(parameters=invalid_parameter_names)


def test_hnsw_validation() -> None:
with pytest.raises(ValueError, match="must be less than or equal"):
HNSWConfiguration(batch_size=500, sync_threshold=100)

0 comments on commit 81cb3cf

Please sign in to comment.