-
Notifications
You must be signed in to change notification settings - Fork 7
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
base: update.redisearch
Are you sure you want to change the base?
Conversation
Accepted pull request to modify WS_SEARCH to SEARCH_WINDOW_SIZE. Should we still use WS_SEARCH? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds more detailed logging output to the experiment runner to improve visibility into experiment progress. The changes primarily focus on displaying memory usage statistics during uploads and search performance metrics during query execution.
- Adds memory usage and vector index size logging during data upload
- Prints search precision and RPS (requests per second) statistics during query execution
- Updates the
ef
parameter extraction to check forWS_SEARCH
as a fallback whenef
is not found
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=}") |
There was a problem hiding this comment.
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.
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.
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=}") |
There was a problem hiding this comment.
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.
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.
@@ -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 |
There was a problem hiding this comment.
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.
ef = calibration_value |
Copilot uses AI. Check for mistakes.
This makes it easier to see how the experiments are progressing.