fix: build IVF_HNSW_SQ graphs with CAGRA - #8411
Conversation
|
Blocked: current-head CI cannot complete because three required jobs failed on transient external downloads before any repository build command ran. Remote head 00444b2 is affected. Rust Clippy and Fmt Check exhausted the mold download with repeated “No data received” responses; Python Linux x86_64 wheel failed while installing maturin with curl error 52, “Empty reply from server”; and windows-build failed while downloading protoc because the connection closed unexpectedly. I inspected all three job logs and confirmed that none reached a project compilation or test failure, while the main Rust format/clippy, Python lint, Java 11/17/21/25, and other platform builds passed, so no speculative code change or push was made. The smallest next action is to rerun these three failed jobs after GitHub download connectivity recovers. As an alternative, rerun their parent workflows; if a rerun reaches project compilation and exposes a source failure, the persistent FixAgent can continue from that evidence. |
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
CAGRA is a reasonable mechanism for the real CPU-only graph-build gap, but the produced graph must preserve Lance's existing SQ metric and v3 HNSW representation. A complete revision should keep topology construction and serialized edge data aligned with the CPU/query contract, then cover a successful accelerated build with recall and released-reader compatibility.
| .chunks_exact(graph_degree) | ||
| .map(|neighbors| { | ||
| let neighbors = Arc::new(neighbors.to_vec()); | ||
| GraphBuilderNode::from_parts(vec![neighbors.clone()], vec![Vec::new()], neighbors) |
There was a problem hiding this comment.
This imports neighbor IDs but leaves level_neighbors_ranked empty, so to_batch() writes populated __neighbors lists with empty _distance lists. I exercised the successful FFI path with an ABI-compatible fake cuVS library on this head: the first row had 2 neighbors and 0 distance values. Released v3 readers before the zero-copy loader rebuild adjacency by zipping these two lists, producing zero edges. Compute one SQ distance per imported edge (using the same VectorStore metric) and add a released-reader/fixture regression so v3 remains forward-compatible.
| fn cuvs_distance_type(distance_type: DistanceType) -> Result<c_int> { | ||
| match distance_type { | ||
| DistanceType::L2 => Ok(0), | ||
| DistanceType::Cosine => Ok(2), |
There was a problem hiding this comment.
SQ's cosine contract is normalized L2: SQDistCalculator ranks both L2 and Cosine with scaled l2_u8, but this maps the CAGRA build to CosineExpanded. Quantization means reconstructed vectors are not exactly unit length, so the order can invert. In a focused head test, codes q=(255,127), A=(11,179), and B=(11,73) gave Lance squared-L2 A < B (62240 < 62452) but cosine A > B (1.91620 > 1.90412). CAGRA can therefore build edges under a different metric than HNSW later searches. Map SQ cosine to L2Expanded and add a cosine recall regression.
| def test_create_index_cagra_accelerator_dispatch(tmp_path, caplog, monkeypatch): | ||
| dataset_module = importlib.import_module("lance.dataset") | ||
| monkeypatch.setattr( | ||
| dataset_module, "_locate_cuvs_library", lambda: "/missing/libcuvs_c.so" |
There was a problem hiding this comment.
This test supplies /missing/libcuvs_c.so, so it covers only loader failure and CPU fallback. It would still pass if the successful CAGRA path never worked; none of the new tests runs a successful cuVS call or checks search quality. Add a successful backend-invocation test plus L2/cosine/dot recall assertions (>=0.5 per repository standard); a deterministic fake C ABI can cover plumbing while a GPU-backed integration test covers actual cuVS behavior.
Summary
Root cause
IVF_HNSW_SQ had no CAGRA integration. The Python accelerator dispatch therefore either entered an IVF_PQ-specific path that produced batches without the source vector column or, after the earlier workaround, discarded the accelerator and built HNSW entirely on CPU.
Validation
Fixes #5061