Skip to content

Print more stats; print WS_SEARCH as ef #29

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
wants to merge 1 commit into
base: update.redisearch
Choose a base branch
from
Open
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
10 changes: 10 additions & 0 deletions engine/base_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ def run_experiment(
upload_stats = self.uploader.upload(
distance=dataset.config.distance, records=reader.read_data(upload_start_idx,upload_end_idx)
)
memory_usage = upload_stats["memory_usage"]
if (used_memory := memory_usage.get("used_memory")) is not None:
print(f"{used_memory=}")
if (index_info := memory_usage.get("index_info")) is not None:
if (vector_index_sz_mb := index_info.get("vector_index_sz_mb")) is not None:
print(f"{vector_index_sz_mb=}")
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The walrus operator assignment in the condition will fail if memory_usage is None or not a dictionary. Add a null check for memory_usage before calling .get() method.

Suggested change
print(f"{vector_index_sz_mb=}")
if isinstance(memory_usage, dict):
if (used_memory := memory_usage.get("used_memory")) is not None:
print(f"{used_memory=}")
if (index_info := memory_usage.get("index_info")) is not None:
if (vector_index_sz_mb := index_info.get("vector_index_sz_mb")) is not None:
print(f"{vector_index_sz_mb=}")

Copilot uses AI. Check for mistakes.

Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

This code assumes memory_usage is a dictionary, but it could be None or a different type. The same null check issue applies here as with the used_memory access above.

Suggested change
print(f"{vector_index_sz_mb=}")
if isinstance(memory_usage, dict):
if (used_memory := memory_usage.get("used_memory")) is not None:
print(f"{used_memory=}")
if (index_info := memory_usage.get("index_info")) is not None:
if (vector_index_sz_mb := index_info.get("vector_index_sz_mb")) is not None:
print(f"{vector_index_sz_mb=}")

Copilot uses AI. Check for mistakes.


if not DETAILED_RESULTS:
# Remove verbose stats from upload results
Expand Down Expand Up @@ -157,6 +163,8 @@ def run_experiment(
ef = "default"
if "search_params" in search_params:
ef = search_params["search_params"].get("ef", "default")
if ef == "default":
ef = search_params["search_params"].get("WS_SEARCH", "default")
client_count = search_params.get("parallel", 1)

# Filter by client count if parallels is specified
Expand Down Expand Up @@ -186,6 +194,7 @@ def run_experiment(
f"Calibrated {top=} {precision=} {calibration_value=} {calibration_precision=!s}"
)
searcher.search_params["search_params"][calibration_param] = calibration_value
ef = calibration_value
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The ef variable is being overwritten with calibration_value but this assignment appears to serve no purpose since ef is only used for printing in the loop and this assignment happens after the print statement. This could be a logic error.

Suggested change
ef = calibration_value

Copilot uses AI. Check for mistakes.


for repetition in range(1, REPETITIONS + 1):
print(
Expand All @@ -195,6 +204,7 @@ def run_experiment(
search_stats = searcher.search_all(
dataset.config.distance, reader.read_queries(), num_queries
)
print(f"{search_stats['mean_precisions']=!s}, {search_stats['rps']=}")
# ensure we specify the client count in the results
search_params["parallel"] = client_count
if not DETAILED_RESULTS:
Expand Down