[feat](eval_performance): support filtered HGraph workloads in AutoTune - #2736
[feat](eval_performance): support filtered HGraph workloads in AutoTune#2736jac0626 wants to merge 1 commit into
Conversation
Signed-off-by: jc543239 <jc543239@antgroup.com> Assisted-by: Codex:gpt-5
|
/label status/waiting-for-review |
Merge Protections🟢 All 3 merge protections satisfied — ready to merge. Show 3 satisfied protections🟢 Require kind label
🟢 Require version label
🟢 Require linked issue for feature/bug PRs
|
There was a problem hiding this comment.
Pull request overview
This PR extends the eval_performance AutoTune typed API to support filtered HGraph workloads by accepting per-query FilterPtr or exclusion BitsetPtr inputs, routing them through the shared evaluation pipeline while keeping HGraph’s native filtered-search overloads, and documenting the new capability.
Changes:
- Extend typed AutoTune workload/request plumbing to carry per-query
FilterPtr/BitsetPtrintoEvalDataset, validation, and evaluation. - Update search evaluation to apply per-query filters/bitsets, and restrict filtered workloads to HGraph (including rejecting
use_extra_info_filter=true). - Add regression tests, new bitset example, and English/Chinese documentation updates.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/eval/eval_dataset.h | Adds per-query filter/bitset storage + accessors and filtered-query accounting. |
| tools/eval/eval_dataset.cpp | Validates filter/bitset inputs and wires them into dataset construction. |
| tools/eval/eval_dataset_test.cpp | Adds coverage for per-query filters/bitsets and input validation. |
| tools/eval/case/search_eval_case.cpp | Applies per-query filter/bitset during KNN evaluation via HGraph overloads. |
| tools/autotune/autotune.h | Extends Workload with query_filters and query_invalid_bitsets. |
| tools/autotune/autotune.cpp | Propagates filtered workload data into eval datasets; enforces HGraph-only for filtered cases; reports filtered count. |
| tools/autotune/autotune_internal.h | Tracks filtered-workload presence in request context. |
| tools/autotune/autotune_candidate.cpp | Rejects extra-info filtering when tuning filtered workloads. |
| tools/autotune/autotune_test.cpp | Adds end-to-end tuning tests for per-query filters/bitsets and validation failures. |
| examples/cpp/CMakeLists.txt | Registers a new bitset-based AutoTune example target. |
| examples/cpp/327_feature_autotune_existing_index.cpp | Extends existing example to demonstrate per-query FilterPtr tuning and usage. |
| examples/cpp/330_feature_autotune_existing_index_bitset.cpp | New example demonstrating per-query exclusion bitsets in AutoTune + final search. |
| docs/docs/zh/src/resources/autotune.md | Documents filtered typed HGraph workloads and links both examples (FilterPtr + bitset). |
| docs/docs/en/src/resources/autotune.md | Documents filtered typed HGraph workloads and links both examples (FilterPtr + bitset). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/retest |
| REQUIRE(bitset_result["recall_avg"].get<double>() == 1.0); | ||
| config.enable_recall = false; | ||
| config.enable_qps = true; | ||
|
|
There was a problem hiding this comment.
[suggestion] The bitset recall test verifies recall_avg == 1.0 correctly, but after setting config.enable_recall = false; config.enable_qps = true; on this line, no corresponding EvaluateSearch call is made for the bitset_dataset. The test immediately falls through to the ef_search=0 error-path test using the original dataset (filter-based). This means the bitset path is never exercised under a QPS-only configuration.
If the intent was to also verify the bitset workload works correctly under QPS measurement, add a call like:
const auto bitset_qps = vsag::eval::EvaluateSearch(index, bitset_dataset, config);
REQUIRE(bitset_qps.contains("qps"));Otherwise, remove the config.enable_qps = true; line to avoid leaving dead state that suggests an incomplete test.
| "filtered workloads currently support only ID filters; " | ||
| "hgraph.use_extra_info_filter must be false"); | ||
|
|
||
| auto index_request = fixture.Request(index_workspace.Get()); |
There was a problem hiding this comment.
[note] The index_request is constructed via fixture.Request(index_workspace.Get()) which initializes workload with the unfiltered ground_truth and a hardcoded top_k=3. The test then immediately overwrites ground_truth with filtered_ground_truth and query_invalid_bitsets. This works because TOP_K (3) coincidentally matches the Request() default, but the dependency on the unfiltered ground truth being set first and then overwritten is fragile.
Consider adding a dedicated factory method (e.g. RequestFiltered) or constructing the IndexRequest directly with the filtered workload fields to make the intent explicit and avoid relying on the unfiltered defaults.
|
|
||
| bool | ||
| CheckValid(int64_t id) const override { | ||
| checks_.fetch_add(1, std::memory_order_relaxed); |
There was a problem hiding this comment.
[note] CheckValid performs a linear scan (std::find) over valid_ids_ on every invocation. For the test fixture this is fine since valid_ids_ is small (size = TOP_K = 3), but if this filter class is ever copied into production code or used with larger allow-lists, the O(n) per-ID check would become a hot-path bottleneck. Consider adding a comment noting this is test-only, or using an unordered_set for the allow-list lookup to make the intent clearer.
LHT129
left a comment
There was a problem hiding this comment.
Thanks for this PR. The implementation is clean and well-structured — the filter/bitset validation, mutual exclusion checks, and index-type gating are all correctly placed. The test coverage is thorough with both positive and negative paths.
I left 3 inline comments:
-
[suggestion]
tools/eval/eval_dataset_test.cpp:556— The bitset QPS test appears incomplete;config.enable_qps = trueis set but never exercised for the bitset dataset. -
[note]
tools/autotune/autotune_test.cpp:1023— The filteredindex_requestconstruction reusesfixture.Request()which sets unfiltered defaults that are immediately overwritten. Consider a dedicated factory to make the intent clearer. -
[note]
tools/autotune/autotune_test.cpp:320—CountingAllowListFilter::CheckValidusesstd::find(O(n)). Fine for the test fixture but worth a comment noting it is test-only.
Overall the PR is in good shape. The core logic (validation, routing, candidate gating) is correct and the documentation updates are thorough.
Change Type
Linked Issue
What Changed
FilterPtrand invalidBitsetPtrworkload inputs to typed HGraphTuneSearchandTuneIndexrequests.FilterPtrandBitsetPtrexamples, focused regression tests, and English/Chinese user documentation.Test Evidence
make fmtmake lintmake testmake cov, run tests, and collect coverageTest details:
Compatibility Impact
tools/; no installed VSAG SDK API is changed.Performance and Concurrency Impact
Documentation Impact
README.mdDEVELOPMENT.mdCONTRIBUTING.mddocs/docs/{en,zh}/src/resources/autotune.mdRisk and Rollback
92dfeaa4; unfiltered AutoTune behavior is otherwise unchanged.Checklist
kind/bugandkind/feature; see "Linked Issue" above)[skip ci]prefix)