Skip to content
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

Add maximum value validation for HNSW parameters #424

Merged
merged 2 commits into from
Apr 5, 2023
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
3 changes: 2 additions & 1 deletion src/marqo/tensor_search/configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def default_env_vars() -> dict:
EnvVars.MARQO_MAX_CONCURRENT_SEARCH: 8,
EnvVars.MARQO_THREAD_EXPIRY_TIME: 1800, # 30 minutes
EnvVars.MARQO_ENABLE_THROTTLING: "TRUE",
EnvVars.MARQO_LOG_LEVEL: "info" # This env variable is set to "info" by default in run_marqo.sh, which overrides this value
EnvVars.MARQO_LOG_LEVEL: "info", # This env variable is set to "info" by default in run_marqo.sh, which overrides this value
EnvVars.MARQO_EF_CONSTRUCTION_MAX_VALUE: 4096,
}

2 changes: 1 addition & 1 deletion src/marqo/tensor_search/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class EnvVars:
MARQO_ENABLE_THROTTLING = "MARQO_ENABLE_THROTTLING"
MARQO_LOG_LEVEL = "MARQO_LOG_LEVEL"
MARQO_ROOT_PATH = "MARQO_ROOT_PATH"

MARQO_EF_CONSTRUCTION_MAX_VALUE = "MARQO_EF_CONSTRUCTION_MAX_VALUE"

class RequestType:
INDEX = "INDEX"
Expand Down
5 changes: 4 additions & 1 deletion src/marqo/tensor_search/models/settings_object.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from marqo.tensor_search import enums as ns_enums
from marqo.tensor_search.enums import IndexSettingsField as NsFields, EnvVars
from marqo.tensor_search.utils import read_env_vars_and_defaults

settings_schema = {
"$schema": "https://json-schema.org/draft/2019-09/schema",
Expand Down Expand Up @@ -127,13 +128,15 @@
NsFields.hnsw_ef_construction: {
"type": "integer",
"minimum": 1,
"maximum": int(read_env_vars_and_defaults(EnvVars.MARQO_EF_CONSTRUCTION_MAX_VALUE)),
"examples": [
128
]
},
NsFields.hnsw_m: {
"type": "integer",
"minimum": 1,
"minimum": 2,
"maximum": 100,
"examples": [
16
]
Expand Down
28 changes: 20 additions & 8 deletions tests/tensor_search/test_create_index.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import pprint
from typing import Any, Dict
import pytest
import os
import requests
from marqo.tensor_search.enums import IndexSettingsField, EnvVars
from marqo.errors import MarqoApiError, MarqoError, IndexNotFoundError
from marqo.tensor_search import tensor_search, configs, backend
from marqo.tensor_search.utils import read_env_vars_and_defaults
from tests.marqo_test import MarqoTestCase
from marqo.tensor_search.enums import IndexSettingsField as NsField
from unittest import mock
Expand Down Expand Up @@ -61,14 +63,25 @@ def test_create_vector_index__invalid_settings(self):
{IndexSettingsField.ann_parameters: {
IndexSettingsField.ann_metric: "innerproduct",
}},
{IndexSettingsField.ann_method_parameters: {
{IndexSettingsField.ann_parameters: {
IndexSettingsField.ann_method_parameters: {
IndexSettingsField.hnsw_ef_construction: 0,
IndexSettingsField.hnsw_m: 16
}},
{IndexSettingsField.ann_method_parameters: {
}}},
{IndexSettingsField.ann_parameters: {
IndexSettingsField.ann_method_parameters: {
IndexSettingsField.hnsw_ef_construction: 128,
IndexSettingsField.hnsw_m: 101
}}},
{IndexSettingsField.ann_parameters: {
IndexSettingsField.ann_method_parameters: {
IndexSettingsField.hnsw_ef_construction: 1 + int(read_env_vars_and_defaults(EnvVars.MARQO_EF_CONSTRUCTION_MAX_VALUE)),
IndexSettingsField.hnsw_m: 16
}}},
{IndexSettingsField.ann_parameters: {IndexSettingsField.ann_method_parameters: {
IndexSettingsField.hnsw_ef_construction: 128,
IndexSettingsField.hnsw_m: -1
}},
}}},
]
for idx_defaults in custom_index_defaults:
with self.subTest(custom_index_defaults=idx_defaults):
Expand All @@ -77,17 +90,16 @@ def test_create_vector_index__invalid_settings(self):
except IndexNotFoundError as s:
pass

try:
with self.assertRaises(errors.InvalidArgError):
print(f"index settings={idx_defaults}")
tensor_search.create_vector_index(
config=self.config,
index_name=self.index_name_1,
index_settings={
NsField.index_defaults: idx_defaults
}
)
raise AssertionError(f"Invalid custom index defaults, {self.custom_index_defaults} , did not cause exception ")
except errors.InvalidArgError as e:
pass
print(tensor_search.get_index_info(self.config, self.index_name_1))

def test_create_vector_index_custom_index_settings(self):
try:
Expand Down