From c73821cf9294a27a1a13f3f629ab5b4a8cdaec0a Mon Sep 17 00:00:00 2001 From: dor-forer Date: Thu, 30 Apr 2026 13:13:15 +0300 Subject: [PATCH 1/9] MOD-15269 Add float16 input support to QuantPreprocessor Refactor QuantPreprocessor so DataType may be either float or vecsim_types::float16. The vector body is stored/queried in DataType width while all SQ8 metadata (min, delta, sum, sum_squares) remains FP32 to match the asymmetric distance kernels added in MOD-15141. - Replace std::is_floating_point_v constraint with an is_quant_input trait that opts in float and float16 explicitly. - Pin metadata to a single MetadataType alias (float) and use it for every value that is written into the metadata region (min/max/diff/ delta, final sum/sum_squares, store_*_metadata signatures, and the return type of find_min_max). Per-element SIMD-lane accumulators remain plain float as a precision choice for FP16 inputs. - Compute storage_bytes_count and query_bytes_count using sizeof(MetadataType) for the metadata region and sizeof(DataType) for the vector body. - Write metadata via memcpy because the metadata offset is not guaranteed to be sizeof(MetadataType)-aligned when DataType is float16 (e.g. odd dim). - Update the class-level documentation to reflect the layout and precision contract. Tests: - Add QuantPreprocessorFP16MetricTest (L2/IP/Cosine) covering blob size, layout, FP16 body fidelity, and FP32 metadata against an FP32 baseline computed from the widened input. - Add QuantPreprocessorFP16Test.QuantizeReconstructRoundTripL2 with an odd dimension (17) verifying |min + delta * q - x| <= delta. - Switch the FP16 fixture's baseline metadata reads to the existing load_meta() memcpy helper, and rewrite ComputeSQ8Quantization to store metadata via memcpy as well. --- src/VecSim/spaces/computer/preprocessors.h | 210 ++++++++++++++------- tests/unit/test_components.cpp | 202 ++++++++++++++++++++ tests/unit/unit_test_utils.h | 11 +- 3 files changed, 345 insertions(+), 78 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 5954b3fc1..4d5b93a1a 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -13,11 +13,14 @@ #include #include #include +#include #include +#include #include "VecSim/memory/vecsim_base.h" #include "VecSim/spaces/spaces.h" #include "VecSim/memory/memory_utils.h" +#include "VecSim/types/float16.h" #include "VecSim/types/sq8.h" class PreprocessorInterface : public VecsimBaseObject { @@ -157,17 +160,19 @@ class CosinePreprocessor : public PreprocessorInterface { * x_sum = Σx_i: sum of the original values, * x_sum_squares = Σx_i²: sum of squares of the original values. * - * The quantized blob size is: - * - For L2: dim * sizeof(OUTPUT_TYPE) + 4 * sizeof(DataType) - * - For IP/Cosine: dim * sizeof(OUTPUT_TYPE) + 3 * sizeof(DataType) + * Storage metadata is always FP32 (independent of DataType) to match the asymmetric distance + * kernels. The quantized blob size is: + * - For L2: dim * sizeof(OUTPUT_TYPE) + 4 * sizeof(float) + * - For IP/Cosine: dim * sizeof(OUTPUT_TYPE) + 3 * sizeof(float) * * Reconstruction formulas: * Given quantized value q_i, the original value is reconstructed as: * x_i ≈ min + delta * q_i * * Query processing: - * The query vector is not quantized. It remains as DataType, but we precompute - * and store metric-specific values to accelerate asymmetric distance computation: + * The query vector is not quantized. It remains in DataType width (FP32 stays FP32, FP16 stays + * FP16), but we precompute and store metric-specific FP32 values to accelerate asymmetric + * distance computation: * - For IP/Cosine: y_sum = Σy_i (sum of query values) * - For L2: y_sum = Σy_i (sum of query values), y_sum_squares = Σy_i² (sum of squared query values) * @@ -175,11 +180,14 @@ class CosinePreprocessor : public PreprocessorInterface { * - For IP/Cosine: | query_values[dim] | y_sum | * - For L2: | query_values[dim] | y_sum | y_sum_squares | * - * Query blob size: - * - For IP/Cosine: (dim + 1) * sizeof(DataType) - * - For L2: (dim + 2) * sizeof(DataType) + * Query metadata is always FP32. The query blob size is: + * - For IP/Cosine: dim * sizeof(DataType) + 1 * sizeof(float) + * - For L2: dim * sizeof(DataType) + 2 * sizeof(float) * - * === Asymmetric distance (storage x quantized, query y remains float) === + * Note: when DataType is float16 the metadata region may not be 4-byte aligned; both writes + * and reads of metadata must therefore go through memcpy. + * + * === Asymmetric distance (storage x quantized, query y in DataType) === * * For IP/Cosine: * IP(x, y) = Σ(x_i * y_i) @@ -217,11 +225,37 @@ class CosinePreprocessor : public PreprocessorInterface { * ||x - y||² = sum_sq_x + sum_sq_y - 2 * IP(x, y) * where sum_sq_x, sum_sq_y are precomputed sums of squared original values. */ +// Trait describing input types accepted by QuantPreprocessor. Specialized below to opt-in +// vecsim_types::float16; rejects unrelated types (e.g. integers, double, bfloat16) by default. +template +struct is_quant_input : std::false_type {}; +template <> +struct is_quant_input : std::true_type {}; +template <> +struct is_quant_input : std::true_type {}; +template +inline constexpr bool is_quant_input_v = is_quant_input::value; + +// Convert a single input element to FP32 for accumulation/comparison. Identity for float, +// FP16 -> FP32 widening for vecsim_types::float16. Templated on the input type. +template +static inline float to_fp32(T x) { + if constexpr (std::is_same_v) { + return vecsim_types::FP16_to_FP32(x); + } else { + static_assert(std::is_same_v, "to_fp32: unsupported input type"); + return x; + } +} + template class QuantPreprocessor : public PreprocessorInterface { using OUTPUT_TYPE = uint8_t; + using MetadataType = float; // SQ8 metadata is always FP32 (see class doc). using sq8 = vecsim_types::sq8; + static_assert(is_quant_input_v, + "QuantPreprocessor only supports float and vecsim_types::float16"); static_assert(Metric == VecSimMetric_L2 || Metric == VecSimMetric_IP || Metric == VecSimMetric_Cosine, "QuantPreprocessor only supports L2, IP and Cosine metrics"); @@ -230,21 +264,21 @@ class QuantPreprocessor : public PreprocessorInterface { // methods. void quantize(const DataType *input, OUTPUT_TYPE *quantized) const { assert(input && quantized); - // Find min and max values + // Find min and max values (computed in MetadataType regardless of DataType). auto [min_val, max_val] = find_min_max(input); - // Calculate scaling factor - const DataType diff = (max_val - min_val); - // Delta = diff / 255.0f - const DataType delta = (diff == DataType{0}) ? DataType{1} : diff / DataType{255}; - const DataType inv_delta = DataType{1} / delta; + // Calculate scaling factor (typed as MetadataType because they end up as metadata). + const MetadataType diff = (max_val - min_val); + const MetadataType delta = (diff == 0.0f) ? MetadataType{1} : diff / MetadataType{255}; + const MetadataType inv_delta = MetadataType{1} / delta; - // Compute sum (and sum of squares for L2) while quantizing + // Compute sum (and sum of squares for L2) while quantizing. + // Accumulators are FP32 to preserve metadata precision for FP16 inputs. // 4 independent accumulators (sum) - DataType s0{}, s1{}, s2{}, s3{}; + float s0{}, s1{}, s2{}, s3{}; // 4 independent accumulators (sum of squares), only used for L2 - DataType q0{}, q1{}, q2{}, q3{}; + float q0{}, q1{}, q2{}, q3{}; size_t i = 0; // round dim down to the nearest multiple of 4 @@ -252,11 +286,11 @@ class QuantPreprocessor : public PreprocessorInterface { // Quantize the values for (; i < dim_round_down; i += 4) { - // Load once - const DataType x0 = input[i + 0]; - const DataType x1 = input[i + 1]; - const DataType x2 = input[i + 2]; - const DataType x3 = input[i + 3]; + // Load once (widened to FP32 if DataType is FP16). + const float x0 = to_fp32(input[i + 0]); + const float x1 = to_fp32(input[i + 1]); + const float x2 = to_fp32(input[i + 2]); + const float x3 = to_fp32(input[i + 3]); // We know (input - min) => 0 // If min == max, all values are the same and should be quantized to 0. // reconstruction will yield the same original value for all vectors. @@ -280,12 +314,13 @@ class QuantPreprocessor : public PreprocessorInterface { } } - // Tail: 0..3 remaining elements (still the same pass, just finishing work) - DataType sum = (s0 + s1) + (s2 + s3); - DataType sum_squares = (q0 + q1) + (q2 + q3); + // Tail: 0..3 remaining elements (still the same pass, just finishing work). + // Sum/sum_squares become metadata, so they are MetadataType. + MetadataType sum = (s0 + s1) + (s2 + s3); + MetadataType sum_squares = (q0 + q1) + (q2 + q3); for (; i < this->dim; ++i) { - const DataType x = input[i]; + const float x = to_fp32(input[i]); quantized[i] = static_cast(std::round((x - min_val) * inv_delta)); sum += x; if constexpr (Metric == VecSimMetric_L2) { @@ -293,37 +328,32 @@ class QuantPreprocessor : public PreprocessorInterface { } } - DataType *metadata = reinterpret_cast(quantized + this->dim); - - // Store min_val, delta, in the metadata - metadata[sq8::MIN_VAL] = min_val; - metadata[sq8::DELTA] = delta; - - // Store sum (for all metrics) and sum_squares (for L2 only) - metadata[sq8::SUM] = sum; - if constexpr (Metric == VecSimMetric_L2) { - metadata[sq8::SUM_SQUARES] = sum_squares; - } + // Metadata uses MetadataType. Use memcpy because the metadata offset + // (dim * sizeof(uint8_t)) is not guaranteed to be 4-byte aligned. + store_storage_metadata(quantized + this->dim, min_val, delta, sum, sum_squares); } - // Computes and assigns query metadata in a single pass over the input vector. - // For IP/Cosine: assigns y_sum = Σy_i - // For L2: assigns y_sum = Σy_i and y_sum_squares = Σy_i² - void assign_query_metadata(const DataType *input, DataType *output_metadata) const { + // Computes and writes query metadata (FP32) in a single pass over the input vector. + // For IP/Cosine: writes y_sum = Σy_i + // For L2: writes y_sum = Σy_i and y_sum_squares = Σy_i² + // The output pointer addresses the metadata region after the query body and may not be + // 4-byte aligned (e.g. FP16 query body with odd dim), so writes go through memcpy. + void assign_query_metadata(const DataType *input, void *output_metadata) const { + // Accumulators are FP32 to preserve precision for FP16 inputs. // 4 independent accumulators for sum - DataType s0{}, s1{}, s2{}, s3{}; + float s0{}, s1{}, s2{}, s3{}; // 4 independent accumulators for sum of squares (only used for L2) - DataType q0{}, q1{}, q2{}, q3{}; + float q0{}, q1{}, q2{}, q3{}; size_t i = 0; // round dim down to the nearest multiple of 4 size_t dim_round_down = this->dim & ~size_t(3); for (; i < dim_round_down; i += 4) { - const DataType y0 = input[i + 0]; - const DataType y1 = input[i + 1]; - const DataType y2 = input[i + 2]; - const DataType y3 = input[i + 3]; + const float y0 = to_fp32(input[i + 0]); + const float y1 = to_fp32(input[i + 1]); + const float y2 = to_fp32(input[i + 2]); + const float y3 = to_fp32(input[i + 3]); s0 += y0; s1 += y1; @@ -338,23 +368,20 @@ class QuantPreprocessor : public PreprocessorInterface { } } - DataType sum = (s0 + s1) + (s2 + s3); - DataType sum_squares = (q0 + q1) + (q2 + q3); + // Sum/sum_squares become metadata, so they are MetadataType. + MetadataType sum = (s0 + s1) + (s2 + s3); + MetadataType sum_squares = (q0 + q1) + (q2 + q3); // Tail: handle remaining elements for (; i < this->dim; ++i) { - const DataType y = input[i]; + const float y = to_fp32(input[i]); sum += y; if constexpr (Metric == VecSimMetric_L2) { sum_squares += y * y; } } - // Assign the computed metadata - output_metadata[sq8::SUM_QUERY] = sum; // y_sum for all metrics - if constexpr (Metric == VecSimMetric_L2) { - output_metadata[sq8::SUM_SQUARES_QUERY] = sum_squares; // y_sum_squares for L2 only - } + store_query_metadata(output_metadata, sum, sum_squares); } public: @@ -362,12 +389,10 @@ class QuantPreprocessor : public PreprocessorInterface { : PreprocessorInterface(allocator), dim(dim), storage_bytes_count(dim * sizeof(OUTPUT_TYPE) + (vecsim_types::sq8::storage_metadata_count()) * - sizeof(DataType)), - query_bytes_count((dim + vecsim_types::sq8::query_metadata_count()) * - sizeof(DataType)) { - static_assert(std::is_floating_point_v, - "QuantPreprocessor only supports floating-point types"); - } + sizeof(MetadataType)), + query_bytes_count(dim * sizeof(DataType) + + (vecsim_types::sq8::query_metadata_count()) * + sizeof(MetadataType)) {} void preprocess(const void *original_blob, void *&storage_blob, void *&query_blob, size_t &input_blob_size, unsigned char alignment) const override { @@ -434,7 +459,7 @@ class QuantPreprocessor : public PreprocessorInterface { /** * Preprocesses the query vector for asymmetric distance computation. * - * The query blob contains the original float values followed by precomputed values: + * The query blob contains the original DataType values followed by FP32 precomputed values: * - For IP/Cosine: y_sum = Σy_i (sum of query values) * - For L2: y_sum = Σy_i (sum of query values), y_sum_squares = Σy_i² (sum of squared query * values) @@ -444,8 +469,8 @@ class QuantPreprocessor : public PreprocessorInterface { * - For L2: | query_values[dim] | y_sum | y_sum_squares | * * Query blob size: - * - For IP/Cosine: (dim + 1) * sizeof(DataType) - * - For L2: (dim + 2) * sizeof(DataType) + * - For IP/Cosine: dim * sizeof(DataType) + 1 * sizeof(float) + * - For L2: dim * sizeof(DataType) + 2 * sizeof(float) */ void preprocessQuery(const void *original_blob, void *&blob, size_t &query_blob_size, unsigned char alignment) const override { @@ -453,12 +478,14 @@ class QuantPreprocessor : public PreprocessorInterface { // Allocate aligned memory for the query blob blob = this->allocator->allocate_aligned(this->query_bytes_count, alignment); - memcpy(blob, original_blob, this->dim * sizeof(DataType)); + const size_t body_bytes = this->dim * sizeof(DataType); + memcpy(blob, original_blob, body_bytes); const DataType *input = static_cast(original_blob); - DataType *output = static_cast(blob); - // Compute and assign query metadata (sum for IP/Cosine, sum and sum_squares for L2) - assign_query_metadata(input, output + this->dim); + // Compute and write FP32 query metadata after the query body. The metadata offset is + // body_bytes, which is not guaranteed to be 4-byte aligned for FP16 query bodies. + void *metadata_dst = static_cast(blob) + body_bytes; + assign_query_metadata(input, metadata_dst); query_blob_size = this->query_bytes_count; } @@ -473,9 +500,48 @@ class QuantPreprocessor : public PreprocessorInterface { } private: - std::pair find_min_max(const DataType *input) const { - auto [min_it, max_it] = std::minmax_element(input, input + dim); - return {*min_it, *max_it}; + // Returns (min, max) of the input vector evaluated in MetadataType. Required because float16 + // does not have a usable operator< (it implicitly converts to uint16_t, ignoring the sign + // bit). The returned values are written verbatim into the metadata region. + std::pair find_min_max(const DataType *input) const { + if constexpr (std::is_same_v) { + MetadataType min_val = to_fp32(input[0]); + MetadataType max_val = min_val; + for (size_t i = 1; i < dim; ++i) { + const MetadataType v = to_fp32(input[i]); + if (v < min_val) min_val = v; + if (v > max_val) max_val = v; + } + return {min_val, max_val}; + } else { + auto [min_it, max_it] = std::minmax_element(input, input + dim); + return {static_cast(*min_it), static_cast(*max_it)}; + } + } + + // Writes the SQ8 storage metadata at the given destination. Uses memcpy because the + // base pointer (quantized + dim) is not guaranteed to be sizeof(MetadataType)-aligned. + static void store_storage_metadata(void *dst, MetadataType min_val, MetadataType delta, + MetadataType sum, MetadataType sum_squares) { + const MetadataType buf_ip_cos[3] = {min_val, delta, sum}; + const MetadataType buf_l2[4] = {min_val, delta, sum, sum_squares}; + if constexpr (Metric == VecSimMetric_L2) { + memcpy(dst, buf_l2, sizeof(buf_l2)); + } else { + memcpy(dst, buf_ip_cos, sizeof(buf_ip_cos)); + } + } + + // Writes the query metadata at the given destination. Uses memcpy for the same + // alignment reason as store_storage_metadata. + static void store_query_metadata(void *dst, MetadataType sum, MetadataType sum_squares) { + const MetadataType buf_ip_cos[1] = {sum}; + const MetadataType buf_l2[2] = {sum, sum_squares}; + if constexpr (Metric == VecSimMetric_L2) { + memcpy(dst, buf_l2, sizeof(buf_l2)); + } else { + memcpy(dst, buf_ip_cos, sizeof(buf_ip_cos)); + } } const size_t dim; diff --git a/tests/unit/test_components.cpp b/tests/unit/test_components.cpp index 2f91d2411..7ce49f4eb 100644 --- a/tests/unit/test_components.cpp +++ b/tests/unit/test_components.cpp @@ -1295,3 +1295,205 @@ INSTANTIATE_TEST_SUITE_P(QuantPreprocessorTests, QuantPreprocessorMetricTest, [](const testing::TestParamInfo &info) { return VecSimMetric_ToString(info.param); }); + +// Parameterized test class for QuantPreprocessor. Verifies the hybrid layout: +// storage = [uint8 * dim][float * N], query = [float16 * dim][float * M], with FP32 metadata +// matching the FP32-quantized baseline of the same input widened to FP32. +class QuantPreprocessorFP16MetricTest : public testing::TestWithParam { +protected: + using float16 = vecsim_types::float16; + static constexpr size_t dim = 5; + static constexpr unsigned char alignment = 0; + static constexpr size_t original_blob_size = dim * sizeof(float16); + + std::shared_ptr allocator; + float16 original_blob[dim]; // FP16 input + float widened_blob[dim]; // FP32 view of the same input (round-trip through FP16) + + void SetUp() override { + allocator = VecSimAllocator::newVecsimAllocator(); + const float src[dim] = {1.0f, 2.0f, 3.0f, 4.0f, 5.0f}; + for (size_t i = 0; i < dim; ++i) { + original_blob[i] = vecsim_types::FP32_to_FP16(src[i]); + widened_blob[i] = vecsim_types::FP16_to_FP32(original_blob[i]); + } + } + + template + static size_t getExpectedStorageSize() { + return dim * sizeof(uint8_t) + sq8::storage_metadata_count() * sizeof(float); + } + + template + static size_t getExpectedQuerySize() { + return dim * sizeof(float16) + sq8::query_metadata_count() * sizeof(float); + } + + // Reads an FP32 metadata scalar at the given byte offset from `base` via memcpy (the + // metadata region is not guaranteed to be 4-byte aligned for FP16 query bodies). + static float load_meta(const void *base, size_t byte_offset) { + float v; + std::memcpy(&v, static_cast(base) + byte_offset, sizeof(float)); + return v; + } + + template + void runQuantizationTest() { + const size_t expected_storage_size = getExpectedStorageSize(); + const size_t expected_query_size = getExpectedQuerySize(); + const size_t storage_meta_offset = dim * sizeof(uint8_t); + const size_t query_meta_offset = dim * sizeof(float16); + + // FP32 baseline: quantize the widened (FP16->FP32) input through the same algorithm. + // Read metadata via load_meta() because baseline_storage is a uint8_t buffer and the + // metadata region (offset = dim) is not guaranteed to be 4-byte aligned. + constexpr size_t max_storage_size = dim * sizeof(uint8_t) + 4 * sizeof(float); + uint8_t baseline_storage[max_storage_size]; + ComputeSQ8Quantization(widened_blob, dim, baseline_storage); + const float baseline_min = load_meta(baseline_storage, dim + sq8::MIN_VAL * sizeof(float)); + const float baseline_delta = load_meta(baseline_storage, dim + sq8::DELTA * sizeof(float)); + const float baseline_sum = load_meta(baseline_storage, dim + sq8::SUM * sizeof(float)); + const float baseline_sum_sq = + load_meta(baseline_storage, dim + sq8::SUM_SQUARES * sizeof(float)); + + auto quant_preprocessor = + new (allocator) QuantPreprocessor(allocator, dim); + + // Test preprocess (both storage and query) + { + void *storage_blob = nullptr; + void *query_blob = nullptr; + size_t storage_blob_size = original_blob_size; + size_t query_blob_size = original_blob_size; + + quant_preprocessor->preprocess(original_blob, storage_blob, query_blob, + storage_blob_size, query_blob_size, alignment); + + // Verify storage blob layout/size + ASSERT_NE(storage_blob, nullptr); + ASSERT_EQ(storage_blob_size, expected_storage_size); + + // Verify query blob layout/size + ASSERT_NE(query_blob, nullptr); + ASSERT_EQ(query_blob_size, expected_query_size); + + // Storage quantized values must match the FP32 baseline. + EXPECT_NO_FATAL_FAILURE(CompareVectors( + static_cast(storage_blob), baseline_storage, dim)); + + // Storage FP32 metadata must match the baseline values. + ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::MIN_VAL * sizeof(float)), + baseline_min); + ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::DELTA * sizeof(float)), + baseline_delta); + ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::SUM * sizeof(float)), + baseline_sum); + if constexpr (Metric == VecSimMetric_L2) { + ASSERT_FLOAT_EQ(load_meta(storage_blob, + storage_meta_offset + sq8::SUM_SQUARES * sizeof(float)), + baseline_sum_sq); + } + + // Query body must be a bit-equal copy of the FP16 input. + EXPECT_NO_FATAL_FAILURE(CompareVectors( + static_cast(query_blob), original_blob, dim)); + + // Query FP32 metadata: y_sum (and y_sum_squares for L2) match the FP32 baseline. + ASSERT_FLOAT_EQ(load_meta(query_blob, + query_meta_offset + sq8::SUM_QUERY * sizeof(float)), + baseline_sum); + if constexpr (Metric == VecSimMetric_L2) { + ASSERT_FLOAT_EQ( + load_meta(query_blob, + query_meta_offset + sq8::SUM_SQUARES_QUERY * sizeof(float)), + baseline_sum_sq); + } + + allocator->free_allocation(storage_blob); + allocator->free_allocation(query_blob); + } + + // Test preprocessQuery alone. + { + void *blob = nullptr; + size_t blob_size = original_blob_size; + quant_preprocessor->preprocessQuery(original_blob, blob, blob_size, alignment); + + ASSERT_NE(blob, nullptr); + ASSERT_EQ(blob_size, expected_query_size); + EXPECT_NO_FATAL_FAILURE( + CompareVectors(static_cast(blob), original_blob, dim)); + ASSERT_FLOAT_EQ(load_meta(blob, query_meta_offset + sq8::SUM_QUERY * sizeof(float)), + baseline_sum); + if constexpr (Metric == VecSimMetric_L2) { + ASSERT_FLOAT_EQ( + load_meta(blob, query_meta_offset + sq8::SUM_SQUARES_QUERY * sizeof(float)), + baseline_sum_sq); + } + allocator->free_allocation(blob); + } + + delete quant_preprocessor; + } +}; + +TEST_P(QuantPreprocessorFP16MetricTest, QuantizationBlobSizeAndMetadata) { + VecSimMetric metric = GetParam(); + switch (metric) { + case VecSimMetric_L2: + runQuantizationTest(); + break; + case VecSimMetric_IP: + runQuantizationTest(); + break; + case VecSimMetric_Cosine: + runQuantizationTest(); + break; + } +} + +INSTANTIATE_TEST_SUITE_P(QuantPreprocessorFP16Tests, QuantPreprocessorFP16MetricTest, + testing::Values(VecSimMetric_L2, VecSimMetric_IP, VecSimMetric_Cosine), + [](const testing::TestParamInfo &info) { + return VecSimMetric_ToString(info.param); + }); + +// Quantize -> reconstruct round-trip for FP16 input. Verifies that for each quantized value +// q_i, reconstructed = min + delta * q_i is within one quantization step of the original +// FP16 value (widened to FP32). Also covers the in-place quantization path. +TEST(QuantPreprocessorFP16Test, QuantizeReconstructRoundTripL2) { + using float16 = vecsim_types::float16; + auto allocator = VecSimAllocator::newVecsimAllocator(); + constexpr size_t dim = 17; // odd, exercises the tail loop and unaligned metadata writes + const float src[dim] = {-3.5f, -2.0f, -1.25f, -0.5f, -0.125f, 0.0f, 0.125f, 0.5f, 1.0f, + 1.5f, 2.0f, 2.5f, 3.0f, 3.25f, 3.4f, 3.45f, 3.5f}; + float16 input[dim]; + float widened[dim]; + for (size_t i = 0; i < dim; ++i) { + input[i] = vecsim_types::FP32_to_FP16(src[i]); + widened[i] = vecsim_types::FP16_to_FP32(input[i]); + } + + auto preprocessor = + new (allocator) QuantPreprocessor(allocator, dim); + + void *storage_blob = nullptr; + size_t storage_blob_size = 0; + preprocessor->preprocessForStorage(input, storage_blob, storage_blob_size); + ASSERT_NE(storage_blob, nullptr); + ASSERT_EQ(storage_blob_size, dim * sizeof(uint8_t) + 4 * sizeof(float)); + + const uint8_t *quantized = static_cast(storage_blob); + float min_val, delta; + std::memcpy(&min_val, quantized + dim + sq8::MIN_VAL * sizeof(float), sizeof(float)); + std::memcpy(&delta, quantized + dim + sq8::DELTA * sizeof(float), sizeof(float)); + + // Reconstruction error should be bounded by the quantization step (delta). + for (size_t i = 0; i < dim; ++i) { + const float reconstructed = min_val + delta * static_cast(quantized[i]); + EXPECT_NEAR(reconstructed, widened[i], delta); + } + + allocator->free_allocation(storage_blob); + delete preprocessor; +} diff --git a/tests/unit/unit_test_utils.h b/tests/unit/unit_test_utils.h index a61682902..519a130f1 100644 --- a/tests/unit/unit_test_utils.h +++ b/tests/unit/unit_test_utils.h @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -250,12 +251,10 @@ inline void ComputeSQ8Quantization(const float *original_blob, size_t dim, uint8 sum_squares += original_blob[i] * original_blob[i]; } - // Store metadata: min_val, delta, sum, sum_squares - float *metadata = reinterpret_cast(output + dim); - metadata[sq8::MIN_VAL] = min_val; - metadata[sq8::DELTA] = delta; - metadata[sq8::SUM] = sum; - metadata[sq8::SUM_SQUARES] = sum_squares; + // Store metadata: min_val, delta, sum, sum_squares. Use memcpy because the metadata region + // (output + dim) is not guaranteed to be 4-byte aligned for arbitrary dim values. + const float metadata[4] = {min_val, delta, sum, sum_squares}; + std::memcpy(output + dim, metadata, sizeof(metadata)); } // TODO: Move all test_utils to this namespace From e1871513464bbcf4dacc7b858010eeda2b2e09fd Mon Sep 17 00:00:00 2001 From: dor-forer Date: Thu, 30 Apr 2026 13:17:04 +0300 Subject: [PATCH 2/9] MOD-15269 Cover in-place quantization in FP16 round-trip test The test comment claimed it 'covers the in-place quantization path' but only called preprocessForStorage. Add an actual preprocessStorageInPlace call: seed a buffer sized to max(input_size, storage_size) with the FP16 input and verify the resulting SQ8 blob byte-for-byte matches the one produced by preprocessForStorage. --- tests/unit/test_components.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/test_components.cpp b/tests/unit/test_components.cpp index 7ce49f4eb..ff0e503b4 100644 --- a/tests/unit/test_components.cpp +++ b/tests/unit/test_components.cpp @@ -1494,6 +1494,19 @@ TEST(QuantPreprocessorFP16Test, QuantizeReconstructRoundTripL2) { EXPECT_NEAR(reconstructed, widened[i], delta); } + // In-place path: seed a buffer large enough to hold both the FP16 input and the SQ8 + // storage layout, copy the FP16 input in, and quantize in place. The resulting SQ8 blob + // must match the one produced by preprocessForStorage. + constexpr size_t input_size = dim * sizeof(float16); + constexpr size_t storage_size = dim * sizeof(uint8_t) + 4 * sizeof(float); + constexpr size_t buf_size = (input_size > storage_size) ? input_size : storage_size; + alignas(float) uint8_t in_place_buf[buf_size]{}; + std::memcpy(in_place_buf, input, input_size); + preprocessor->preprocessStorageInPlace(in_place_buf, buf_size); + EXPECT_NO_FATAL_FAILURE( + CompareVectors(in_place_buf, static_cast(storage_blob), + storage_size)); + allocator->free_allocation(storage_blob); delete preprocessor; } From cb597dc38b23df5e3effcdd50f67ed43ad97e4b8 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Thu, 30 Apr 2026 13:22:14 +0300 Subject: [PATCH 3/9] Refactor code for improved readability in preprocessors and test components --- src/VecSim/spaces/computer/preprocessors.h | 6 ++- tests/unit/test_components.cpp | 43 +++++++++++----------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 4d5b93a1a..14675a568 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -509,8 +509,10 @@ class QuantPreprocessor : public PreprocessorInterface { MetadataType max_val = min_val; for (size_t i = 1; i < dim; ++i) { const MetadataType v = to_fp32(input[i]); - if (v < min_val) min_val = v; - if (v > max_val) max_val = v; + if (v < min_val) + min_val = v; + if (v > max_val) + max_val = v; } return {min_val, max_val}; } else { diff --git a/tests/unit/test_components.cpp b/tests/unit/test_components.cpp index ff0e503b4..ecbe8a84d 100644 --- a/tests/unit/test_components.cpp +++ b/tests/unit/test_components.cpp @@ -1307,8 +1307,8 @@ class QuantPreprocessorFP16MetricTest : public testing::TestWithParam allocator; - float16 original_blob[dim]; // FP16 input - float widened_blob[dim]; // FP32 view of the same input (round-trip through FP16) + float16 original_blob[dim]; // FP16 input + float widened_blob[dim]; // FP32 view of the same input (round-trip through FP16) void SetUp() override { allocator = VecSimAllocator::newVecsimAllocator(); @@ -1382,16 +1382,18 @@ class QuantPreprocessorFP16MetricTest : public testing::TestWithParam(storage_blob), baseline_storage, dim)); // Storage FP32 metadata must match the baseline values. - ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::MIN_VAL * sizeof(float)), - baseline_min); - ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::DELTA * sizeof(float)), - baseline_delta); + ASSERT_FLOAT_EQ( + load_meta(storage_blob, storage_meta_offset + sq8::MIN_VAL * sizeof(float)), + baseline_min); + ASSERT_FLOAT_EQ( + load_meta(storage_blob, storage_meta_offset + sq8::DELTA * sizeof(float)), + baseline_delta); ASSERT_FLOAT_EQ(load_meta(storage_blob, storage_meta_offset + sq8::SUM * sizeof(float)), baseline_sum); if constexpr (Metric == VecSimMetric_L2) { - ASSERT_FLOAT_EQ(load_meta(storage_blob, - storage_meta_offset + sq8::SUM_SQUARES * sizeof(float)), - baseline_sum_sq); + ASSERT_FLOAT_EQ( + load_meta(storage_blob, storage_meta_offset + sq8::SUM_SQUARES * sizeof(float)), + baseline_sum_sq); } // Query body must be a bit-equal copy of the FP16 input. @@ -1399,14 +1401,13 @@ class QuantPreprocessorFP16MetricTest : public testing::TestWithParam(query_blob), original_blob, dim)); // Query FP32 metadata: y_sum (and y_sum_squares for L2) match the FP32 baseline. - ASSERT_FLOAT_EQ(load_meta(query_blob, - query_meta_offset + sq8::SUM_QUERY * sizeof(float)), - baseline_sum); + ASSERT_FLOAT_EQ( + load_meta(query_blob, query_meta_offset + sq8::SUM_QUERY * sizeof(float)), + baseline_sum); if constexpr (Metric == VecSimMetric_L2) { - ASSERT_FLOAT_EQ( - load_meta(query_blob, - query_meta_offset + sq8::SUM_SQUARES_QUERY * sizeof(float)), - baseline_sum_sq); + ASSERT_FLOAT_EQ(load_meta(query_blob, query_meta_offset + + sq8::SUM_SQUARES_QUERY * sizeof(float)), + baseline_sum_sq); } allocator->free_allocation(storage_blob); @@ -1466,7 +1467,7 @@ TEST(QuantPreprocessorFP16Test, QuantizeReconstructRoundTripL2) { auto allocator = VecSimAllocator::newVecsimAllocator(); constexpr size_t dim = 17; // odd, exercises the tail loop and unaligned metadata writes const float src[dim] = {-3.5f, -2.0f, -1.25f, -0.5f, -0.125f, 0.0f, 0.125f, 0.5f, 1.0f, - 1.5f, 2.0f, 2.5f, 3.0f, 3.25f, 3.4f, 3.45f, 3.5f}; + 1.5f, 2.0f, 2.5f, 3.0f, 3.25f, 3.4f, 3.45f, 3.5f}; float16 input[dim]; float widened[dim]; for (size_t i = 0; i < dim; ++i) { @@ -1474,8 +1475,7 @@ TEST(QuantPreprocessorFP16Test, QuantizeReconstructRoundTripL2) { widened[i] = vecsim_types::FP16_to_FP32(input[i]); } - auto preprocessor = - new (allocator) QuantPreprocessor(allocator, dim); + auto preprocessor = new (allocator) QuantPreprocessor(allocator, dim); void *storage_blob = nullptr; size_t storage_blob_size = 0; @@ -1503,9 +1503,8 @@ TEST(QuantPreprocessorFP16Test, QuantizeReconstructRoundTripL2) { alignas(float) uint8_t in_place_buf[buf_size]{}; std::memcpy(in_place_buf, input, input_size); preprocessor->preprocessStorageInPlace(in_place_buf, buf_size); - EXPECT_NO_FATAL_FAILURE( - CompareVectors(in_place_buf, static_cast(storage_blob), - storage_size)); + EXPECT_NO_FATAL_FAILURE(CompareVectors( + in_place_buf, static_cast(storage_blob), storage_size)); allocator->free_allocation(storage_blob); delete preprocessor; From 532b4f341944caa4c1126378faa8000273f981e5 Mon Sep 17 00:00:00 2001 From: dor-forer Date: Thu, 30 Apr 2026 14:45:07 +0300 Subject: [PATCH 4/9] MOD-15269 Replace is_quant_input trait with QuantInput concept Use a C++20 named concept on the QuantPreprocessor template head and on the to_fp32 helper instead of an is_quant_input trait combined with a static_assert. The constraint is now checked at the template's instantiation point with a named diagnostic, and the helper trait machinery is dropped. --- src/VecSim/spaces/computer/preprocessors.h | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 14675a568..8e3c629fb 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -225,37 +226,28 @@ class CosinePreprocessor : public PreprocessorInterface { * ||x - y||² = sum_sq_x + sum_sq_y - 2 * IP(x, y) * where sum_sq_x, sum_sq_y are precomputed sums of squared original values. */ -// Trait describing input types accepted by QuantPreprocessor. Specialized below to opt-in -// vecsim_types::float16; rejects unrelated types (e.g. integers, double, bfloat16) by default. +// Input types accepted by QuantPreprocessor. Opt-in via std::same_as so unrelated types +// (e.g. integers, double, bfloat16) are rejected at the template head with a named constraint. template -struct is_quant_input : std::false_type {}; -template <> -struct is_quant_input : std::true_type {}; -template <> -struct is_quant_input : std::true_type {}; -template -inline constexpr bool is_quant_input_v = is_quant_input::value; +concept QuantInput = std::same_as || std::same_as; // Convert a single input element to FP32 for accumulation/comparison. Identity for float, -// FP16 -> FP32 widening for vecsim_types::float16. Templated on the input type. -template +// FP16 -> FP32 widening for vecsim_types::float16. +template static inline float to_fp32(T x) { if constexpr (std::is_same_v) { return vecsim_types::FP16_to_FP32(x); } else { - static_assert(std::is_same_v, "to_fp32: unsupported input type"); return x; } } -template +template class QuantPreprocessor : public PreprocessorInterface { using OUTPUT_TYPE = uint8_t; using MetadataType = float; // SQ8 metadata is always FP32 (see class doc). using sq8 = vecsim_types::sq8; - static_assert(is_quant_input_v, - "QuantPreprocessor only supports float and vecsim_types::float16"); static_assert(Metric == VecSimMetric_L2 || Metric == VecSimMetric_IP || Metric == VecSimMetric_Cosine, "QuantPreprocessor only supports L2, IP and Cosine metrics"); From c810e6ad8a12cc4a5759f7758a981dfa1912ebab Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Thu, 30 Apr 2026 15:13:19 +0300 Subject: [PATCH 5/9] Refactor QuantPreprocessor to use 'auto' for type inference in FP32 conversion --- src/VecSim/spaces/computer/preprocessors.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 8e3c629fb..3807bb305 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -279,10 +279,10 @@ class QuantPreprocessor : public PreprocessorInterface { // Quantize the values for (; i < dim_round_down; i += 4) { // Load once (widened to FP32 if DataType is FP16). - const float x0 = to_fp32(input[i + 0]); - const float x1 = to_fp32(input[i + 1]); - const float x2 = to_fp32(input[i + 2]); - const float x3 = to_fp32(input[i + 3]); + const auto x0 = to_fp32(input[i + 0]); + const auto x1 = to_fp32(input[i + 1]); + const auto x2 = to_fp32(input[i + 2]); + const auto x3 = to_fp32(input[i + 3]); // We know (input - min) => 0 // If min == max, all values are the same and should be quantized to 0. // reconstruction will yield the same original value for all vectors. @@ -312,7 +312,7 @@ class QuantPreprocessor : public PreprocessorInterface { MetadataType sum_squares = (q0 + q1) + (q2 + q3); for (; i < this->dim; ++i) { - const float x = to_fp32(input[i]); + const auto x = to_fp32(input[i]); quantized[i] = static_cast(std::round((x - min_val) * inv_delta)); sum += x; if constexpr (Metric == VecSimMetric_L2) { @@ -342,10 +342,10 @@ class QuantPreprocessor : public PreprocessorInterface { size_t dim_round_down = this->dim & ~size_t(3); for (; i < dim_round_down; i += 4) { - const float y0 = to_fp32(input[i + 0]); - const float y1 = to_fp32(input[i + 1]); - const float y2 = to_fp32(input[i + 2]); - const float y3 = to_fp32(input[i + 3]); + const auto y0 = to_fp32(input[i + 0]); + const auto y1 = to_fp32(input[i + 1]); + const auto y2 = to_fp32(input[i + 2]); + const auto y3 = to_fp32(input[i + 3]); s0 += y0; s1 += y1; @@ -366,7 +366,7 @@ class QuantPreprocessor : public PreprocessorInterface { // Tail: handle remaining elements for (; i < this->dim; ++i) { - const float y = to_fp32(input[i]); + const auto y = to_fp32(input[i]); sum += y; if constexpr (Metric == VecSimMetric_L2) { sum_squares += y * y; From b987af23dfb86b5ffaba3d165fe37a97827037e2 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Thu, 30 Apr 2026 15:19:09 +0300 Subject: [PATCH 6/9] Refactor QuantPreprocessor to use explicit float type for FP32 conversion --- src/VecSim/spaces/computer/preprocessors.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 3807bb305..8e3c629fb 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -279,10 +279,10 @@ class QuantPreprocessor : public PreprocessorInterface { // Quantize the values for (; i < dim_round_down; i += 4) { // Load once (widened to FP32 if DataType is FP16). - const auto x0 = to_fp32(input[i + 0]); - const auto x1 = to_fp32(input[i + 1]); - const auto x2 = to_fp32(input[i + 2]); - const auto x3 = to_fp32(input[i + 3]); + const float x0 = to_fp32(input[i + 0]); + const float x1 = to_fp32(input[i + 1]); + const float x2 = to_fp32(input[i + 2]); + const float x3 = to_fp32(input[i + 3]); // We know (input - min) => 0 // If min == max, all values are the same and should be quantized to 0. // reconstruction will yield the same original value for all vectors. @@ -312,7 +312,7 @@ class QuantPreprocessor : public PreprocessorInterface { MetadataType sum_squares = (q0 + q1) + (q2 + q3); for (; i < this->dim; ++i) { - const auto x = to_fp32(input[i]); + const float x = to_fp32(input[i]); quantized[i] = static_cast(std::round((x - min_val) * inv_delta)); sum += x; if constexpr (Metric == VecSimMetric_L2) { @@ -342,10 +342,10 @@ class QuantPreprocessor : public PreprocessorInterface { size_t dim_round_down = this->dim & ~size_t(3); for (; i < dim_round_down; i += 4) { - const auto y0 = to_fp32(input[i + 0]); - const auto y1 = to_fp32(input[i + 1]); - const auto y2 = to_fp32(input[i + 2]); - const auto y3 = to_fp32(input[i + 3]); + const float y0 = to_fp32(input[i + 0]); + const float y1 = to_fp32(input[i + 1]); + const float y2 = to_fp32(input[i + 2]); + const float y3 = to_fp32(input[i + 3]); s0 += y0; s1 += y1; @@ -366,7 +366,7 @@ class QuantPreprocessor : public PreprocessorInterface { // Tail: handle remaining elements for (; i < this->dim; ++i) { - const auto y = to_fp32(input[i]); + const float y = to_fp32(input[i]); sum += y; if constexpr (Metric == VecSimMetric_L2) { sum_squares += y * y; From 6cb2f0f39a3ba08b58bc96e85478c80165ef8cd3 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Mon, 4 May 2026 12:08:52 +0300 Subject: [PATCH 7/9] MOD-15269 Add FP32-semantic ordering operators to vecsim_types::float16 The implicit conversion to uint16_t made any < / > comparison between two float16 values silently use raw bit-pattern ordering, which is incorrect for IEEE 754: the sign bit is the MSB, so negatives compared as larger than positives, and same-signed values compared correctly only when non-negative. Add operator<=> (returning std::partial_ordering for NaN handling) and operator==, both delegating to FP32 semantics. The exact-match overloads take precedence over the implicit uint16_t conversion path, so all six comparisons now do the right thing. Add unit tests covering the full operator set across negatives, +/-0, and positives, plus a regression test for std::minmax_element on a mix where the min and max are negative. --- src/VecSim/types/float16.h | 13 +++++++++++ tests/unit/test_types.cpp | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/VecSim/types/float16.h b/src/VecSim/types/float16.h index fef2fa0b3..3f5a1c43a 100644 --- a/src/VecSim/types/float16.h +++ b/src/VecSim/types/float16.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace vecsim_types { struct float16 { uint16_t val; @@ -48,6 +49,18 @@ static inline float FP16_to_FP32(float16 input) { return _interpret_as_float(((exp == shifted_exp) ? infnan_val : reg_val) | sign_bit); } +// Comparison operators that delegate to FP32 semantics. Required because the implicit +// conversion to uint16_t would otherwise compare the raw bit pattern, which is not a valid +// ordering for IEEE 754 values (the sign bit is the MSB, so negatives appear "larger" than +// positives, and same-signed magnitudes compare correctly only for non-negative values). +// These exact-match overloads take precedence over the implicit uint16_t conversion path. +inline std::partial_ordering operator<=>(float16 a, float16 b) { + return FP16_to_FP32(a) <=> FP16_to_FP32(b); +} +inline bool operator==(float16 a, float16 b) { + return FP16_to_FP32(a) == FP16_to_FP32(b); +} + static inline float16 FP32_to_FP16(float input) { // via Fabian "ryg" Giesen. // https://gist.github.com/2156668 diff --git a/tests/unit/test_types.cpp b/tests/unit/test_types.cpp index 6d700b143..21703819d 100644 --- a/tests/unit/test_types.cpp +++ b/tests/unit/test_types.cpp @@ -7,10 +7,54 @@ * GNU Affero General Public License v3 (AGPLv3). */ +#include +#include #include #include "gtest/gtest.h" #include "VecSim/types/float16.h" +class FP16TypeCompare : public ::testing::Test {}; + +TEST_F(FP16TypeCompare, OrderingMatchesFP32) { + using vecsim_types::float16; + using vecsim_types::FP32_to_FP16; + + // Mix of negatives, zeros, and positives. Same-magnitude positive vs negative is the + // historically broken case (raw uint16_t compare would put -1.0 above +1.0 because of the + // sign bit). + const std::array values{-2.5f, -1.0f, -0.0f, 0.0f, 0.5f, 1.0f, 2.5f}; + + for (size_t i = 0; i < values.size(); ++i) { + for (size_t j = 0; j < values.size(); ++j) { + const float16 a = FP32_to_FP16(values[i]); + const float16 b = FP32_to_FP16(values[j]); + EXPECT_EQ(a < b, values[i] < values[j]) << values[i] << " < " << values[j]; + EXPECT_EQ(a > b, values[i] > values[j]) << values[i] << " > " << values[j]; + EXPECT_EQ(a <= b, values[i] <= values[j]) << values[i] << " <= " << values[j]; + EXPECT_EQ(a >= b, values[i] >= values[j]) << values[i] << " >= " << values[j]; + EXPECT_EQ(a == b, values[i] == values[j]) << values[i] << " == " << values[j]; + EXPECT_EQ(a != b, values[i] != values[j]) << values[i] << " != " << values[j]; + } + } +} + +TEST_F(FP16TypeCompare, MinmaxElementHandlesNegatives) { + using vecsim_types::float16; + using vecsim_types::FP16_to_FP32; + using vecsim_types::FP32_to_FP16; + + // Min and max are both negative, with the min having a larger absolute value. Under the + // pre-fix uint16_t-based comparison, the negative with the larger magnitude would have + // compared as the maximum, swapping the result. + const std::array data{FP32_to_FP16(-3.5f), FP32_to_FP16(0.5f), + FP32_to_FP16(-1.25f), FP32_to_FP16(2.0f), + FP32_to_FP16(-0.75f)}; + + auto [min_it, max_it] = std::minmax_element(data.begin(), data.end()); + EXPECT_FLOAT_EQ(FP16_to_FP32(*min_it), -3.5f); + EXPECT_FLOAT_EQ(FP16_to_FP32(*max_it), 2.0f); +} + #ifdef OPT_AVX512_FP16_VL class FP16Type : public ::testing::Test {}; From 50723a0083c2143aaedff4cbcc015510920c0656 Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Mon, 4 May 2026 12:09:03 +0300 Subject: [PATCH 8/9] MOD-15269 Simplify QuantPreprocessor metadata writes Two cleanups in QuantPreprocessor, both enabled by recent groundwork: 1. find_min_max no longer needs the float16-specific manual loop; with the new operator<=> on vecsim_types::float16, std::minmax_element works correctly for both DataTypes. The function collapses to a single line shared by both branches. 2. Inline store_storage_metadata and store_query_metadata into their only call sites. The original helpers always built both the L2 and IP/Cosine metadata buffers and then memcpy'd one; the inlined form constructs only the buffer that's actually written. --- src/VecSim/spaces/computer/preprocessors.h | 71 ++++++++-------------- 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/src/VecSim/spaces/computer/preprocessors.h b/src/VecSim/spaces/computer/preprocessors.h index 8e3c629fb..a51f54ed9 100644 --- a/src/VecSim/spaces/computer/preprocessors.h +++ b/src/VecSim/spaces/computer/preprocessors.h @@ -321,8 +321,15 @@ class QuantPreprocessor : public PreprocessorInterface { } // Metadata uses MetadataType. Use memcpy because the metadata offset - // (dim * sizeof(uint8_t)) is not guaranteed to be 4-byte aligned. - store_storage_metadata(quantized + this->dim, min_val, delta, sum, sum_squares); + // (dim * sizeof(uint8_t)) is not guaranteed to be sizeof(MetadataType)-aligned. + void *meta_dst = quantized + this->dim; + if constexpr (Metric == VecSimMetric_L2) { + const MetadataType buf[4] = {min_val, delta, sum, sum_squares}; + memcpy(meta_dst, buf, sizeof(buf)); + } else { + const MetadataType buf[3] = {min_val, delta, sum}; + memcpy(meta_dst, buf, sizeof(buf)); + } } // Computes and writes query metadata (FP32) in a single pass over the input vector. @@ -373,7 +380,16 @@ class QuantPreprocessor : public PreprocessorInterface { } } - store_query_metadata(output_metadata, sum, sum_squares); + // Metadata uses MetadataType. Use memcpy because the metadata offset (after the query + // body of dim * sizeof(DataType)) is not guaranteed to be sizeof(MetadataType)-aligned + // when DataType is float16 and dim is odd. + if constexpr (Metric == VecSimMetric_L2) { + const MetadataType buf[2] = {sum, sum_squares}; + memcpy(output_metadata, buf, sizeof(buf)); + } else { + const MetadataType buf[1] = {sum}; + memcpy(output_metadata, buf, sizeof(buf)); + } } public: @@ -492,50 +508,13 @@ class QuantPreprocessor : public PreprocessorInterface { } private: - // Returns (min, max) of the input vector evaluated in MetadataType. Required because float16 - // does not have a usable operator< (it implicitly converts to uint16_t, ignoring the sign - // bit). The returned values are written verbatim into the metadata region. + // Returns (min, max) of the input vector evaluated in MetadataType. Both float and float16 + // expose a usable operator< (float16's overload delegates to FP32 semantics), so a single + // std::minmax_element call covers both DataTypes. The returned values are written verbatim + // into the metadata region. std::pair find_min_max(const DataType *input) const { - if constexpr (std::is_same_v) { - MetadataType min_val = to_fp32(input[0]); - MetadataType max_val = min_val; - for (size_t i = 1; i < dim; ++i) { - const MetadataType v = to_fp32(input[i]); - if (v < min_val) - min_val = v; - if (v > max_val) - max_val = v; - } - return {min_val, max_val}; - } else { - auto [min_it, max_it] = std::minmax_element(input, input + dim); - return {static_cast(*min_it), static_cast(*max_it)}; - } - } - - // Writes the SQ8 storage metadata at the given destination. Uses memcpy because the - // base pointer (quantized + dim) is not guaranteed to be sizeof(MetadataType)-aligned. - static void store_storage_metadata(void *dst, MetadataType min_val, MetadataType delta, - MetadataType sum, MetadataType sum_squares) { - const MetadataType buf_ip_cos[3] = {min_val, delta, sum}; - const MetadataType buf_l2[4] = {min_val, delta, sum, sum_squares}; - if constexpr (Metric == VecSimMetric_L2) { - memcpy(dst, buf_l2, sizeof(buf_l2)); - } else { - memcpy(dst, buf_ip_cos, sizeof(buf_ip_cos)); - } - } - - // Writes the query metadata at the given destination. Uses memcpy for the same - // alignment reason as store_storage_metadata. - static void store_query_metadata(void *dst, MetadataType sum, MetadataType sum_squares) { - const MetadataType buf_ip_cos[1] = {sum}; - const MetadataType buf_l2[2] = {sum, sum_squares}; - if constexpr (Metric == VecSimMetric_L2) { - memcpy(dst, buf_l2, sizeof(buf_l2)); - } else { - memcpy(dst, buf_ip_cos, sizeof(buf_ip_cos)); - } + auto [min_it, max_it] = std::minmax_element(input, input + dim); + return {to_fp32(*min_it), to_fp32(*max_it)}; } const size_t dim; From c95341f9eba56dda6675111b51e0e31baa1c308f Mon Sep 17 00:00:00 2001 From: Dor Forer Date: Mon, 4 May 2026 12:12:53 +0300 Subject: [PATCH 9/9] Refactor comparison operators for float16 to improve readability and maintain consistency --- src/VecSim/types/float16.h | 4 +--- tests/unit/test_types.cpp | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/VecSim/types/float16.h b/src/VecSim/types/float16.h index 3f5a1c43a..74847e23c 100644 --- a/src/VecSim/types/float16.h +++ b/src/VecSim/types/float16.h @@ -57,9 +57,7 @@ static inline float FP16_to_FP32(float16 input) { inline std::partial_ordering operator<=>(float16 a, float16 b) { return FP16_to_FP32(a) <=> FP16_to_FP32(b); } -inline bool operator==(float16 a, float16 b) { - return FP16_to_FP32(a) == FP16_to_FP32(b); -} +inline bool operator==(float16 a, float16 b) { return FP16_to_FP32(a) == FP16_to_FP32(b); } static inline float16 FP32_to_FP16(float input) { // via Fabian "ryg" Giesen. diff --git a/tests/unit/test_types.cpp b/tests/unit/test_types.cpp index 21703819d..9f0127ac8 100644 --- a/tests/unit/test_types.cpp +++ b/tests/unit/test_types.cpp @@ -46,9 +46,8 @@ TEST_F(FP16TypeCompare, MinmaxElementHandlesNegatives) { // Min and max are both negative, with the min having a larger absolute value. Under the // pre-fix uint16_t-based comparison, the negative with the larger magnitude would have // compared as the maximum, swapping the result. - const std::array data{FP32_to_FP16(-3.5f), FP32_to_FP16(0.5f), - FP32_to_FP16(-1.25f), FP32_to_FP16(2.0f), - FP32_to_FP16(-0.75f)}; + const std::array data{FP32_to_FP16(-3.5f), FP32_to_FP16(0.5f), FP32_to_FP16(-1.25f), + FP32_to_FP16(2.0f), FP32_to_FP16(-0.75f)}; auto [min_it, max_it] = std::minmax_element(data.begin(), data.end()); EXPECT_FLOAT_EQ(FP16_to_FP32(*min_it), -3.5f);