Arnej/quantized vector strong types - #37366
Draft
arnej27959 wants to merge 5 commits into
Draft
Conversation
Replace raw std::span<uint8_t> in the EdenQuantizer API with dedicated QuantizedVector/MutableQuantizedVector and PackedBits/MutablePackedBits view types (composition over std::span, not inheritance). These encapsulate the [f32 scale][packed bits] buffer layout so callers no longer do sizeof(float) byte arithmetic and can't confuse a raw buffer, a quantized vector, and the packed bits it contains. The former free helpers extract_scale_factor and packed_bits_buf are gone; that logic now lives on the view types. All callers in vespalib and searchlib are updated to wrap their byte buffers at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the strong-type treatment to MultiBitPacker: the packed side of pack()/unpack() now takes MutablePackedBits/PackedBits instead of raw uint8_t pointers, and the redundant `n` argument is dropped (derived from the value span, since it cannot be recovered from the packed span due to last-byte padding). A debug assert checks the packed span is large enough. The __restrict__ optimization is preserved via local raw-pointer aliases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the generic packed-bits view types out of quantized_vector.h into a dedicated packed_bits.h. This keeps the layering clean: the generic MultiBitPacker now depends only on packed_bits.h instead of the EDEN-specific quantized_vector.h, while quantized_vector.h reuses the same PackedBits type by including the new header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The quant library and its own tests live inside namespace vespalib::quant and already reference the new strong types unqualified. The search::tensor consumers repeated the fully-qualified vespalib::quant::QuantizedVector / MutableQuantizedVector spellings; add targeted using-declarations (matching each file's existing using-decl convention, not using-namespace) so the call sites read cleanly without polluting the global namespace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vekterli
reviewed
Jul 7, 2026
| } | ||
| }; | ||
| using QuantizedVector = basic_quantized_vector<const uint8_t>; | ||
| using MutableQuantizedVector = basic_quantized_vector<uint8_t>; |
Member
There was a problem hiding this comment.
I think we should make it clear that these are views and not resource-owning value types. E.g. basic_quantized_vector_view, QuantizedVectorView etc.
| [[nodiscard]] constexpr std::span<Byte> span() const noexcept { return _bits; } | ||
| }; | ||
| using PackedBits = basic_packed_bits<const uint8_t>; | ||
| using MutablePackedBits = basic_packed_bits<uint8_t>; |
Member
There was a problem hiding this comment.
I think these should also preferably be named something with "view" to make semantics clearer
Comment on lines
+38
to
+52
| [[nodiscard]] float scale() const noexcept { | ||
| float s; | ||
| memcpy(&s, _buf.data(), scale_bytes); | ||
| return s; | ||
| } | ||
| // Writes the f32 scale factor to the head of the buffer. Only available on mutable views. | ||
| void set_scale(float s) const noexcept | ||
| requires(!std::is_const_v<Byte>) | ||
| { | ||
| memcpy(_buf.data(), &s, scale_bytes); | ||
| } | ||
| // The sub-view holding just the bit-packed centroid indexes (i.e. everything past the scale factor). | ||
| [[nodiscard]] constexpr basic_packed_bits<Byte> packed_bits() const noexcept { | ||
| return basic_packed_bits<Byte>(_buf.subspan(scale_bytes)); | ||
| } |
Member
There was a problem hiding this comment.
This introduces a very leaky abstraction on how quantization is implemented internally. The intention was that outside the actual (de-)quantizer code you only see quantized tensors as entirely opaque byte arrays whose contents you should not make any assumptions about.
Encapsulating these opaque bytes in strong types is a Good Thing™ which I think makes sense, but I don't think they should expose anything else than the fact that it's a quantized tensor wrapping some particular memory area.
Rework the dot-product helpers so they take strongly-typed QuantizedVector views instead of raw int8_t*/uint8_t* pointers built via ad-hoc reinterpret_cast + manual span construction at each call site. Add small as_quantized() converters (from TypedCells and from a span of Int8Float) plus an adapt_lhs() helper that centralizes the "raw f32 query vs. pre-quantized i8 lhs" choice in one place. Why: continues the quantized-vector strong-types migration. Making the pointer-vs-quantized distinction explicit in the overload set removes the scattered casts and repeated quantized_size() plumbing, so the type system now enforces that quantized dot products receive quantized vectors.
Member
|
Note that I'm currently working right in the middle of the code that this modifies fairly extensively, so it might be a good idea to hold this off for a couple of days until the dust settles a bit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
as this is basically a mechanical transform of the existing code, I let claude do the detail work; let's discuss this as a sketch/proposal.