feat: add ivf_pq support to milvus - #846
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: FantasqueX The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| db=DBTYPE, | ||
| db_config=_build_milvus_config(parameters), | ||
| db_case_config=_with_partition_key( | ||
| IVFPQConfig( |
There was a problem hiding this comment.
vectordb_bench/backend/clients/milvus/cli.py line:369
Low ---- This adds a new user-facing command and manually maps seven IVF_PQ/refine options into the Milvus config, but no test exercises this path. The repository already has tests/test_milvus_zilliz_cli.py with a CliRunner/captured-run seam; please add a case there that invokes MilvusIVFPQ and asserts both the index and search payloads, so option-name/default regressions do not silently change the benchmark request.
There was a problem hiding this comment.
I have added a test case.
Signed-off-by: Letu Ren <letur@nvidia.com>
jamesgao-jpg
left a comment
jamesgao-jpg
left a comment
There was a problem hiding this comment.
One benchmark-default compatibility issue found.
| IndexType.HNSW_PQ.value, | ||
| IndexType.HNSW_PRQ.value, | ||
| IndexType.IVF_RABITQ.value, | ||
| IndexType.IVFPQ.value, |
There was a problem hiding this comment.
[P1] Preserve the existing non-refined IVF_PQ default
CaseConfigParamInput_Refine uses options: [True, False], and inputWidget passes those options to st.selectbox without an explicit index, so True is selected by default. Adding IVFPQ here therefore enables FP32 refinement for an untouched frontend IVF_PQ run even though IVFPQConfig defines refine=False and the previous frontend behavior was non-refined. This silently changes the benchmark workload and makes default results incomparable. Please give IVF_PQ a selector that defaults to False without changing the existing HNSW/IVF_RABITQ defaults, and add a focused frontend-default regression test.
4 participants
I don't see similar tests around so not add tests. If you want, I can add them.