feat(index): default ivf_rq to 5-bit quantization - #8936
Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
Five bits is an evidence-backed quality/cost knee, and the core cross-language changes behave as intended, but this revision breaks an explicit namespace override and leaves operational sizing documentation describing the old default. A complete revision should preserve the requested bit width at every entry point and document the new default resource trade-off with the 1-bit opt-out.
| fn default() -> Self { | ||
| Self { | ||
| num_bits: 1, | ||
| num_bits: RABIT_DEFAULT_NUM_BITS, |
There was a problem hiding this comment.
CreateTableIndexRequest.num_bits is never applied in DirectoryNamespace::build_index_params: the IVF_RQ branch constructs RQBuildParams::default(). Once this line makes that default 5, a namespace request that explicitly asks for num_bits=1 silently builds 5-bit codes, violating explicit-value preservation and adding four code bits per dimension. Please apply the request value with checked conversion and 1..=9 validation in the namespace branch, then cover omitted and explicit values.
Reproducer run against this head
I added this unit test inside the existing dir.rs test module and ran CARGO_TARGET_DIR=/home/agent/tmp/gate8936-repro.9RQaq9/target cargo test -p lance-namespace-impls repro_namespace_ivf_rq_preserves_explicit_num_bits --lib --locked:
#[test]
fn repro_namespace_ivf_rq_preserves_explicit_num_bits() {
let mut request = CreateTableIndexRequest::new(
"vector".to_string(),
"IVF_RQ".to_string(),
);
request.num_bits = Some(1);
let DirectoryIndexParams::Vector { params, .. } =
DirectoryNamespace::build_index_params(&request).unwrap()
else {
panic!("expected vector index parameters");
};
let actual = params.stages.iter().find_map(|stage| match stage {
lance::index::vector::StageParams::RQ(rq) => Some(rq.num_bits),
_ => None,
}).unwrap();
assert_eq!(actual, 1);
}The assertion failed with left: 5, right: 1.
There was a problem hiding this comment.
Fixed in 9d01458d2: namespace IVF_RQ creation now preserves explicit values and rejects values outside 1..=9; the omitted, explicit-1, maximum, and invalid cases all pass.
|
|
||
| - num_bits | ||
| The number of bits for RQ (Rabit Quantization). Default is 1. | ||
| The number of bits for RQ (Rabit Quantization). Default is 5. |
There was a problem hiding this comment.
This changes the API default to 5, but docs/src/guide/performance.md still says RQ is 1-bit and sizes it as dimension / 8 + 16 bytes per row. The current 5-bit raw-query writer stores the sign code, four extra bits per dimension, five f32 factors, and the row ID; for the documented 100M × 768 example that is 508 bytes per row, about 47.3 GiB instead of 10.8 GiB. Please update the operational sizing guidance and describe the storage/build/search trade-off plus the explicit num_bits=1 opt-out so users can capacity-plan the new default.
There was a problem hiding this comment.
Fixed in 9d01458d2: the guide now sizes the 5-bit default at 508 bytes per 768-dimensional row (about 47.3 GiB per 100M rows) and documents the 1-bit opt-out plus its build, storage, and search trade-off.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
Both earlier findings are fixed: namespace creation now preserves explicit IVF_RQ bit widths with bounded validation, and the capacity guide reflects the 5-bit layout and the 1-bit opt-out. The 5-bit default remains supported by the established recall and latency trade-off, with no remaining acceptance blocker.
What changed
num_bitsvalues with checked conversion and 1..=9 validation while keeping omitted values at the 5-bit default.Why
Implicit IVF_RQ index creation should consistently use
num_bits=5across supported language surfaces, while explicitnum_bitsvalues must remain unchanged. Capacity guidance must also reflect the larger multi-bit layout so users can choose the 1-bit opt-out when appropriate.Validation
Static checks and formatting completed locally:
cargo fmt --all -- --checkcargo clippy -p lance-index --tests -- -D warningscargo clippy -p lance-namespace-impls --tests -- -D warningsruff,ruff-format,fmt, andtyposThe complete test suite is delegated to CI. Java validation was not run locally because this host does not have a JDK.