Update cuvs-bench docs to show both CLI and Python API usage#2084
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds Python API documentation examples to the benchmark guide and refactors ConfigLoader.load() into a template-method flow that discovers backend algo groups, expands build/search parameter grids, and delegates BenchmarkConfig construction to backend-specific hooks (with CppGBench updated accordingly). ChangesProgrammatic Benchmark Alternatives
Orchestrator ConfigLoader refactor
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
…oader calls expand_param_grid directly
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py`:
- Around line 533-557: The code currently uses tune_mode as a blanket flag when
calling prepare_indexes even if tune_build_params or tune_search_params are
missing, causing validation to be skipped for fallback grids; change the call
site so that prepare_indexes only receives tune_mode for the specific lists that
came from Optuna: compute booleans (e.g., build_from_tune = tune_mode and
tune_build_params is not None; search_from_tune = tune_mode and
tune_search_params is not None), set actual_build/actual_search as you already
do, and modify the prepare_indexes call to accept and use these finer-grained
flags (or pass separate parameters like tune_build_flag and tune_search_flag) so
validation runs for the fallback build_combos/search_combos while being skipped
only for the real tuned param lists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7cbdab07-950d-4a0d-a260-08cb56e697b5
📒 Files selected for processing (1)
python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py
…, not on fallback grids.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py (1)
525-549:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTune flow still uses a blanket validation bypass.
build_from_tuneis passed astune_modeintoprepare_indexes(), which skips both build and search validation. This still bypasses search validation when only build params are tuned. Split build/search skip flags and only skip the side that actually came from Optuna.💡 Minimal fix
- build_from_tune = tune_mode and tune_build_params is not None - if build_from_tune: + skip_build_validation = ( + tune_mode and tune_build_params is not None + ) + skip_search_validation = ( + tune_mode and tune_search_params is not None + ) + + if skip_build_validation: actual_build = [tune_build_params.copy()] - actual_search = ( - [tune_search_params.copy()] - if tune_search_params - else [{}] - ) else: actual_build = build_combos - actual_search = search_combos + + if skip_search_validation: + actual_search = [tune_search_params.copy()] + else: + actual_search = search_combos indexes = self.prepare_indexes( actual_build, actual_search, @@ - tune_mode=build_from_tune, + skip_build_validation=skip_build_validation, + skip_search_validation=skip_search_validation, )- tune_mode: bool = False, + skip_build_validation: bool = False, + skip_search_validation: bool = False, @@ - if not tune_mode: + if not skip_build_validation: if not self.validate_constraints( @@ - if tune_mode: + if skip_search_validation: index["search_params"] = list(all_search_params) else: index["search_params"] = self.validate_search_params(As per coding guidelines, "Ensure missing validation does not cause crashes on invalid input through proper size/type checks".
Also applies to: 786-856
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py` around lines 525 - 549, The tune flow currently passes a single tune_mode (build_from_tune) into prepare_indexes which causes both build and search validations to be skipped; change the logic to compute two separate flags (e.g., build_tune_mode = bool(tune_build_params) and search_tune_mode = bool(tune_search_params)), use actual_build/actual_search as before, and update the prepare_indexes signature and all its call sites (including the other occurrence noted) to accept and use these two flags so only the side coming from Optuna skips validation; inside prepare_indexes, ensure it only bypasses build validation when build_tune_mode is true and only bypasses search validation when search_tune_mode is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py`:
- Around line 525-549: The tune flow currently passes a single tune_mode
(build_from_tune) into prepare_indexes which causes both build and search
validations to be skipped; change the logic to compute two separate flags (e.g.,
build_tune_mode = bool(tune_build_params) and search_tune_mode =
bool(tune_search_params)), use actual_build/actual_search as before, and update
the prepare_indexes signature and all its call sites (including the other
occurrence noted) to accept and use these two flags so only the side coming from
Optuna skips validation; inside prepare_indexes, ensure it only bypasses build
validation when build_tune_mode is true and only bypasses search validation when
search_tune_mode is true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 408ce488-f8a5-4da4-94f9-16f1329a36b6
📒 Files selected for processing (1)
python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py
cjnolet
approved these changes
May 14, 2026
cjnolet
added
doc
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py (1)
530-537: 💤 Low valueRename unused loop variable
group_confto_group_conf.The variable is unpacked but never referenced in the loop body. Prefix with underscore to signal intentional disuse.
for ( algo, group, - group_conf, + _group_conf, build_combos, search_combos, group_meta, ) in expanded_groups:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py` around lines 530 - 537, The unpacking in the for-loop over expanded_groups uses the variable name group_conf which is never referenced; rename it to _group_conf in the tuple unpacking (the for (algo, group, _group_conf, build_combos, search_combos, group_meta) in expanded_groups:) to indicate intentional unused variable and silence linters while leaving surrounding logic (algo, group, build_combos, search_combos, group_meta) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py`:
- Around line 530-537: The unpacking in the for-loop over expanded_groups uses
the variable name group_conf which is never referenced; rename it to _group_conf
in the tuple unpacking (the for (algo, group, _group_conf, build_combos,
search_combos, group_meta) in expanded_groups:) to indicate intentional unused
variable and silence linters while leaving surrounding logic (algo, group,
build_combos, search_combos, group_meta) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 698351c6-0833-4b84-8d51-a48a66a17aa2
📒 Files selected for processing (1)
python/cuvs_bench/cuvs_bench/orchestrator/config_loaders.py
Member
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.