Skip to content

[RFC] Bring your own type erasure - #1068

Merged
Mark Hildebrand (hildebrandmw) merged 6 commits into
mainfrom
mhildebr/byo-type-erasure
Jul 2, 2026
Merged

[RFC] Bring your own type erasure#1068
Mark Hildebrand (hildebrandmw) merged 6 commits into
mainfrom
mhildebr/byo-type-erasure

Conversation

@hildebrandmw

@hildebrandmw Mark Hildebrand (hildebrandmw) commented May 14, 2026

Copy link
Copy Markdown
Contributor

A small RFC describing a pattern we can use to reduce levels indirection in distance function composition. No true action is needed in the codebase, I'm just writing down what I've recommended to several people for an issue I'm seeing occur more frequently.

Rendered

@hildebrandmw Mark Hildebrand (hildebrandmw) changed the title [RFC] Bring your own type erasure. [RFC] Bring your own type erasure May 14, 2026
@hildebrandmw
Mark Hildebrand (hildebrandmw) marked this pull request as ready for review May 14, 2026 17:57
@hildebrandmw
Mark Hildebrand (hildebrandmw) requested review from a team and Copilot May 14, 2026 17:57

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.

Thanks Mark, this is super helpful.

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 adds an RFC documenting a “bring your own type erasure” pattern intended to reduce nested dispatch in distance-computation composition.

Changes:

  • Adds a new RFC describing the motivation, example pattern, generated-code comparison, and trade-offs.
  • Identifies potential application areas such as DistanceProvider, spherical quantization kernels, and multi-vector backends.
Comments suppressed due to low confidence (3)

rfcs/01068-byo-type-erasure.md:31

  • Grammar: the article and plural noun do not agree here.
The combination of unwrapping + delegation is used to create another trait object, leading to an unavoidable situations where we have at least two levels of dynamic dispatch.

rfcs/01068-byo-type-erasure.md:19

  • Grammar: this compound modifier should be hyphenated before the noun.
Lower level APIs in our library use various flavors of type-erasure to enable polymorphism over metric, micro-architecture, and length specialization.

rfcs/01068-byo-type-erasure.md:44

  • Grammar: this compound modifier should be hyphenated before the noun.
How can we redesign our lower level APIs to allow composition of distance computations?

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

Comment thread rfcs/01068-byo-type-erasure.md Outdated
Comment thread rfcs/01068-byo-type-erasure.md Outdated
Comment thread rfcs/01068-byo-type-erasure.md Outdated
Comment thread rfcs/01068-byo-type-erasure.md Outdated
Comment thread rfcs/01068-byo-type-erasure.md
Comment thread rfcs/01068-byo-type-erasure.md Outdated
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.99%. Comparing base (ccf8d4c) to head (29de7cb).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1068      +/-   ##
==========================================
+ Coverage   89.95%   90.99%   +1.04%     
==========================================
  Files         489      489              
  Lines       93127    93127              
==========================================
+ Hits        83773    84744     +971     
+ Misses       9354     8383     -971     
Flag Coverage Δ
miri 90.99% <ø> (+1.04%) ⬆️
unittests 90.96% <ø> (+1.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 40 files with indirect coverage changes

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

Suryansh Gupta (suri-kumkaran) added a commit that referenced this pull request May 30, 2026
Adds the multi-vector MaxSim distance benchmark to `diskann-benchmark`
behind a `multi-vector` Cargo feature (off by default). Public library
API stays minimal: internals (`Kernel<A>`, `tiled_reduce`, layouts,
`TileBudget`) remain private; the only external surface is the generic
`build_max_sim<T>` factory (gated by the sealed `MaxSimElement` trait)
plus the `MaxSimKernel<T>` / `Erase<T>` traits.

The factory follows the BYOTE ("Bring your own type erasure") pattern
from RFC #1068.

`MaxSim::new` is now infallible (`-> Self`, no longer `-> Result`) and
`MaxSimError::BufferLengthIsZero` is removed. With the kernel boundary
enforcing the only meaningful precondition (`scores.len() ==
nrows()`), an empty `scores` buffer is a defined no-op — the standalone
"can't be empty" rule was defensive code that caught nothing the size
check doesn't already catch.

## JSON shape

```json
{
  "type": "multi-vector-op",
  "content": {
    "element_type": "float32",
    "isa": "x86-64-v3",
    "runs": [ { "num_query_vectors": 8, "num_doc_vectors": 32, "dim": 128, "loops_per_measurement": 200, "num_measurements": 50 } ]
  }
}
```

## Suggested review order

1. **`distance/isa.rs`** — `MaxSimIsa`, `NotSupported`,
`is_available()`.
2. **`distance/kernel.rs`** — `MaxSimKernel<T>` (`Result`-returning),
   `Erase<T>`, `BoxErase`.
3. **`distance/factory.rs`** — sealed `MaxSimElement` + `build_max_sim`;
   `Prepared` (SIMD) and `ReferenceKernel`; parity, size-mismatch,
   zero-doc, and zero-query tests.
4. **`distance/max_sim.rs`** — `MaxSim::new` infallible;
   `BufferLengthIsZero` removed.
5. **`multi_vector/{mod,distance/mod}.rs`** — public re-exports; verify
   `QueryComputer` is gone.
6. **`benchmark/inputs/multi_vector.rs`** — `MultiVectorOp`, `BenchIsa`
   shadow enum, `From<BenchIsa> for MaxSimIsa`.
7. **`benchmark/backend/multi_vector/{mod,driver,kernels}.rs`** —
   `cfg_if!` gate; `Kernel<T>` carrier registered per `MaxSimElement`;
   `try_match` rejects unavailable ISAs at job selection.
8. **Glue + tests** — `backend/mod.rs`, `inputs/mod.rs`, `Cargo.toml`
   feature, `main.rs` integration tests, JSON fixtures.

---------

Co-authored-by: Suryansh Gupta <suryangupta@microsoft.com>
@hildebrandmw
Mark Hildebrand (hildebrandmw) merged commit 2c4a6e2 into main Jul 2, 2026
22 checks passed
@hildebrandmw
Mark Hildebrand (hildebrandmw) deleted the mhildebr/byo-type-erasure branch July 2, 2026 15:11
weiyaoluo (SeliMeli) pushed a commit to SeliMeli/DiskANN that referenced this pull request Jul 22, 2026
Adds the multi-vector MaxSim distance benchmark to `diskann-benchmark`
behind a `multi-vector` Cargo feature (off by default). Public library
API stays minimal: internals (`Kernel<A>`, `tiled_reduce`, layouts,
`TileBudget`) remain private; the only external surface is the generic
`build_max_sim<T>` factory (gated by the sealed `MaxSimElement` trait)
plus the `MaxSimKernel<T>` / `Erase<T>` traits.

The factory follows the BYOTE ("Bring your own type erasure") pattern
from RFC microsoft#1068.

`MaxSim::new` is now infallible (`-> Self`, no longer `-> Result`) and
`MaxSimError::BufferLengthIsZero` is removed. With the kernel boundary
enforcing the only meaningful precondition (`scores.len() ==
nrows()`), an empty `scores` buffer is a defined no-op — the standalone
"can't be empty" rule was defensive code that caught nothing the size
check doesn't already catch.

## JSON shape

```json
{
  "type": "multi-vector-op",
  "content": {
    "element_type": "float32",
    "isa": "x86-64-v3",
    "runs": [ { "num_query_vectors": 8, "num_doc_vectors": 32, "dim": 128, "loops_per_measurement": 200, "num_measurements": 50 } ]
  }
}
```

## Suggested review order

1. **`distance/isa.rs`** — `MaxSimIsa`, `NotSupported`,
`is_available()`.
2. **`distance/kernel.rs`** — `MaxSimKernel<T>` (`Result`-returning),
   `Erase<T>`, `BoxErase`.
3. **`distance/factory.rs`** — sealed `MaxSimElement` + `build_max_sim`;
   `Prepared` (SIMD) and `ReferenceKernel`; parity, size-mismatch,
   zero-doc, and zero-query tests.
4. **`distance/max_sim.rs`** — `MaxSim::new` infallible;
   `BufferLengthIsZero` removed.
5. **`multi_vector/{mod,distance/mod}.rs`** — public re-exports; verify
   `QueryComputer` is gone.
6. **`benchmark/inputs/multi_vector.rs`** — `MultiVectorOp`, `BenchIsa`
   shadow enum, `From<BenchIsa> for MaxSimIsa`.
7. **`benchmark/backend/multi_vector/{mod,driver,kernels}.rs`** —
   `cfg_if!` gate; `Kernel<T>` carrier registered per `MaxSimElement`;
   `try_match` rejects unavailable ISAs at job selection.
8. **Glue + tests** — `backend/mod.rs`, `inputs/mod.rs`, `Cargo.toml`
   feature, `main.rs` integration tests, JSON fixtures.

---------

Co-authored-by: Suryansh Gupta <suryangupta@microsoft.com>
weiyaoluo (SeliMeli) pushed a commit to SeliMeli/DiskANN that referenced this pull request Jul 22, 2026
A small RFC describing a pattern we can use to reduce levels indirection
in distance function composition. No true action is needed in the
codebase, I'm just writing down what I've recommended to several people
for an issue I'm seeing occur more frequently.
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.

7 participants