Skip to content

Strip generic from PQ computers.#1247

Open
hildebrandmw wants to merge 1 commit into
mainfrom
mhildebr/strip-generic
Open

Strip generic from PQ computers.#1247
hildebrandmw wants to merge 1 commit into
mainfrom
mhildebr/strip-generic

Conversation

@hildebrandmw

@hildebrandmw hildebrandmw commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Back in the day, distance and query computers could not carry lifetimes. That has since changed. As such, we can remove the Deref<Target = FixedChunkPQTable> bound from PQ distance computers and simplify things a bit.

This is a breaking change and will require a version bump.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the generic Deref<Target = FixedChunkPQTable> bound from PQ distance/query computers in diskann-providers and switches them to borrow &FixedChunkPQTable via lifetimes, simplifying the API now that lifetimes are supported.

Changes:

  • Refactors PQ distance/query computer types (DistanceComputer, QueryComputer, Multi*) from generic T: Deref<Target = FixedChunkPQTable> to &FixedChunkPQTable + explicit lifetimes.
  • Updates async provider call sites and hybrid PQ pruning adapter types to propagate the new lifetime-based computers.
  • Updates benchmark integration type aliases to match the new QueryComputer<'a> shape.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
diskann-providers/src/model/pq/distance/multi.rs Converts MultiTable/MultiDistanceComputer/MultiQueryComputer to borrow &FixedChunkPQTable via lifetimes; adjusts tests accordingly.
diskann-providers/src/model/pq/distance/l2.rs Refactors TableL2 to store &FixedChunkPQTable instead of a generic deref type.
diskann-providers/src/model/pq/distance/innerproduct.rs Refactors TableIP to store &FixedChunkPQTable instead of a generic deref type.
diskann-providers/src/model/pq/distance/dynamic.rs Refactors QueryComputer and DistanceComputer to be lifetime-based over &FixedChunkPQTable.
diskann-providers/src/model/pq/distance/cosine.rs Refactors DirectCosine to be lifetime-based over &FixedChunkPQTable.
diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs Updates provider helpers to return lifetime-bound PQ computers borrowing the internal table.
diskann-providers/src/model/graph/provider/async_/inmem/product.rs Propagates the new lifetime-based PQ computers through async pruning/accessor plumbing.
diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs Updates provider helpers to return lifetime-bound PQ computers borrowing the internal table.
diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs Updates experimental multi-PQ provider type aliases and constructors for lifetime-based multi-computers.
diskann-providers/src/model/graph/provider/async_/distances.rs Updates hybrid PQ distance adapter to accept the new lifetime-based PQ distance computer.
diskann-benchmark/src/exhaustive/product.rs Updates benchmark quant computer associated type to QueryComputer<'a>.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 531 to +533
/// Test that the table works correctl where there is one inner PQ table.
fn test_distance_computer_multi_with_one<R>(
computer: &MultiDistanceComputer<&'_ FixedChunkPQTable, usize>,
computer: &MultiDistanceComputer<'_, usize>,
@codecov-commenter

codecov-commenter commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (a679dce) to head (fa86f14).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1247   +/-   ##
=======================================
  Coverage   90.12%   90.12%           
=======================================
  Files         504      504           
  Lines       96075    96089   +14     
=======================================
+ Hits        86590    86603   +13     
- Misses       9485     9486    +1     
Flag Coverage Δ
miri 90.12% <100.00%> (+<0.01%) ⬆️
unittests 89.79% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
diskann-benchmark/src/exhaustive/product.rs 100.00% <ø> (ø)
...iders/src/model/graph/provider/async_/distances.rs 58.13% <100.00%> (-2.74%) ⬇️
...aph/provider/async_/experimental/multi_pq_async.rs 96.38% <100.00%> (ø)
...ovider/async_/fast_memory_quant_vector_provider.rs 98.13% <100.00%> (ø)
...s/src/model/graph/provider/async_/inmem/product.rs 90.00% <ø> (ø)
...ph/provider/async_/memory_quant_vector_provider.rs 98.26% <100.00%> (ø)
diskann-providers/src/model/pq/distance/cosine.rs 98.76% <100.00%> (ø)
diskann-providers/src/model/pq/distance/dynamic.rs 96.81% <100.00%> (ø)
...nn-providers/src/model/pq/distance/innerproduct.rs 100.00% <100.00%> (ø)
diskann-providers/src/model/pq/distance/l2.rs 100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants