feat(quantization): expose uniform uint7 and add uint8 by luoxiaojian · Pull Request #608 · alibaba/zvec

luoxiaojian · 2026-07-20T09:05:31Z

No description provided.

Copilot

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR introduces uniform UINT8 quantization end-to-end (converter/reformer, metric, and AVX512-VNNI turbo kernels) and renames the prior “UniformInt8” path to “UniformUint7” while wiring the new quantizers through the public interface.

Changes:

Add UniformUint8StreamingConverter/Reformer, UniformUint8 metric, and AVX512-VNNI distance/quantize kernels (including asymmetric query/record layouts).
Rename/update the existing uniform quantizer/metric/test coverage from UniformInt8 to UniformUint7, and expose new quantizer types in the interface.
Extend turbo dispatch (QuantizeType) and core Index converter selection to support uniform uint7 and uint8.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/core/quantizer/uniform_uint8_reformer_test.cc	New tests validating UINT8 reformer encoding/layout, parameter persistence, and error handling.
tests/core/quantizer/uniform_int8_reformer_test.cc	Updates existing “UniformInt8” tests to the renamed “UniformUint7” APIs and param keys.
tests/core/metric/uniform_uint8_metric_test.cc	New tests for UINT8 metric build distance + asymmetric query scoring and batch behavior.
tests/core/interface/quantizer_param_test.cc	New interface-level serialization test for canonical uniform quantizer names.
src/turbo/turbo.cc	Turbo dispatch updates: add UINT8 kernels, switch uniform path to UINT7, and add a UINT8 quantize func getter.
src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.h	Declares AVX512-VNNI UINT8 distance + batch distance (with optional preprocess).
src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.cc	Implements AVX512-VNNI UINT8 build distance and asymmetric batch query scoring.
src/turbo/distance/avx512_vnni/uniform_uint8/quantize.h	Declares scalar UINT8 quantizer kernel for turbo path.
src/turbo/distance/avx512_vnni/uniform_uint8/quantize.cc	Implements scalar UINT8 quantization (round + clamp to [0,255]).
src/turbo/distance/avx512_vnni/uniform_int8/squared_euclidean.h	Renames uniform distance symbols/comments to UINT7 (file path remains `uniform_int8`).
src/turbo/distance/avx512_vnni/uniform_int8/squared_euclidean.cc	Switches includes/symbol names to UINT7 (file path remains `uniform_int8`).
src/turbo/distance/avx512_vnni/uniform_int8/quantize.h	Renames uniform quantize symbol/comments to UINT7 (file path remains `uniform_int8`).
src/turbo/distance/avx512_vnni/uniform_int8/quantize.cc	Switches includes/symbol name to UINT7 (file path remains `uniform_int8`).
src/include/zvec/turbo/turbo.h	Extends `QuantizeType` with `kUniformUint8` and adds `get_uniform_uint8_quantize_func()`.
src/include/zvec/core/interface/index_param.h	Exposes new interface quantizer enums `kUniformUint7` and `kUniformUint8`.
src/core/quantizer/uniform_uint8_reformer.cc	Adds UINT8 streaming reformer with asymmetric record/query encoding and tail sumsq.
src/core/quantizer/uniform_uint8_converter.cc	Adds UINT8 streaming converter: trains global scale/bias and wraps holders for encoded output.
src/core/quantizer/uniform_int8_reformer.cc	Renames uniform reformer implementation from Int8 to Uint7 and updates param keys/logs.
src/core/quantizer/uniform_int8_converter.cc	Renames uniform converter implementation from Int8 to Uint7, updates meta/params, and restores persisted params.
src/core/quantizer/quantizer_params.h	Replaces `uniform_int8.` keys with `uniform_uint7.` and adds `uniform_uint8.*`.
src/core/metric/uniform_uint8_metric.cc	Adds UINT8 metric with exact build distance and asymmetric query scoring (batch uses turbo when available).
src/core/metric/uniform_int8_metric.cc	Renames UniformInt8 metric to UniformUint7 and updates param keys and factory alias name.
src/core/metric/metric_params.h	Adds uniform uint7/uint8 metric param keys and removes uniform int8 key.
src/core/interface/index_param.cc	Adds `<string_view>` include (context not shown).
src/core/interface/index.cc	Updates quantizer type routing to select UniformUint7/UniformUint8 converters.

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

luoxiaojian · 2026-07-29T09:47:11Z

+  // Global uniform quantization with a shared scale/bias and 128 code values
+  // in [0, 127].
+  kUniformUint7 = 8,
+  // Global uniform quantization with the full uint8 code range [0, 255].
+  kUniformUint8 = 9,


原来的 kUniformInt8 隐式数值就是 8，当前 kUniformUint7 = 8 保留了该数值，kUniformUint8 = 9 是新增值。这个枚举目前没有透传到 Python、DB、Protobuf 或 C API。由于旧名称与实际仅使用 [0,127] 的实现不一致，我们希望趁早直接修正命名，不再保留旧名称的 alias。
我们会补充一行注释，明确说明 8 是为了保留原 kUniformInt8 的数值，避免后续误改。

luoxiaojian · 2026-07-29T10:40:04Z

+    int64_t sum_squared = 0;
+    for (size_t i = 0; i < dimension; ++i) {
+      const int code = static_cast<int>(bytes[i]);
+      sum_squared += code * code;
+      if (shift_output) {
+        bytes[i] = static_cast<uint8_t>(code - 128);
+      }
+    }
+    const int32_t tail = static_cast<int32_t>(sum_squared);
+    std::memcpy(output + dimension, &tail, sizeof(tail));


这个问题确实存在于 Core 支持的最大维度范围内。我们已做如下修改：
将平方和尾部从 int32_t 改为 uint32_t，仍保持 4 字节布局，不改变编码长度。
在最大支持维度 65,536 下，平方和最大为 4,261,478,400，可以由 uint32_t 完整表示。
Core 距离计算使用 int64_t 中间结果；AVX512-VNNI 路径使用无符号 32 位结果及原生无符号浮点转换，避免最终 score 的有符号溢出。
converter、reformer 和 metric 均增加了 65,536 维的上限校验。
新增边界测试，覆盖平方和及查询 score 超过 INT32_MAX 的情况，并验证超出最大维度时返回错误。
这样既解决了溢出问题，也避免了改成 8 字节尾部所带来的存储布局变化。

luoxiaojian · 2026-07-29T10:44:44Z

+#if ZVEC_UNIFORM_UINT8_QUERY_PREPROCESS
+void uniform_squared_euclidean_uint8_preprocessed_batch_distance(
+    const void *const *vectors, const void *query, size_t n, size_t dim,
+    float *distances) {
+  uniform_squared_euclidean_uint8_batch_distance_impl<true>(vectors, query, n,
+                                                            dim, distances);
+}
+
+void uniform_squared_euclidean_uint8_query_preprocess(void *query, size_t dim) {
+  (void)query;
+  (void)dim;
+}
+#endif


当前已删除该宏、空 preprocess hook、preprocessed batch variant 及无效模板参数，并统一使用普通 UINT8 batch kernel。相关测试和 AVX512 编译检查均已通过。

luoxiaojian · 2026-07-29T10:46:37Z

 #include "avx512/rotate/fht/fht.h"
 #include "avx512_vnni/record_quantized_int8/cosine.h"
 #include "avx512_vnni/record_quantized_int8/squared_euclidean.h"
-#include "avx512_vnni/uniform_int8/quantize.h"
-#include "avx512_vnni/uniform_int8/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/quantize.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


已经按方案 (b) 完成了目录和文件的整体 rename

luoxiaojian · 2026-07-29T10:49:21Z

+  int init(const ailego::Params &params) override {
+    float scale = 0.0f;
+    float bias = 0.0f;
+    const bool has_scale = params.get(UNIFORM_UINT8_REFORMER_SCALE, &scale);
+    const bool has_bias = params.get(UNIFORM_UINT8_REFORMER_BIAS, &bias);
+    if (!has_scale && !has_bias) {
+      initialized_ = false;
+      return 0;
+    }


已修复。现在 UniformUint8Reformer::init() 每次都会先通过统一的 Reset() 清理内部状态，包括 initialized_、scale_、bias_、scale_reciprocal_squared_ 和 quantize_func_，cleanup() 也复用相同逻辑。这样无论传入空参数、参数不完整，还是重复执行初始化，都不会保留上一次初始化产生的旧状态。同时补充了“完成有效初始化后再以空参数初始化”的回归测试。

Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/include/zvec/core/interface/index_param.h:1

Although the numeric value is preserved, removing the kUniformInt8 enumerator is a source-breaking API change for downstream code that still references the old name. If backward compatibility is a goal, consider keeping a deprecated alias (e.g., kUniformInt8 = kUniformUint7) and ensuring string (de)serialization accepts both names.

// Copyright 2025-present the zvec project

src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.cc:326

uniform_squared_euclidean_uint8_batch_distance returns early when orig_dim == 0 without writing distances[], leaving the caller with uninitialized outputs. Please mirror the scalar-path behavior by explicitly setting all n distances to 0.0f before returning.

  if (orig_dim == 0) {
    return;
  }

src/core/metric/uniform_uint8_metric.cc:113

Initialization does not validate that meta.dimension() is large enough to contain the uint32 tail (i.e., dimension > sizeof(uint32_t)). As implemented, an undersized dimension makes OriginalDimension() return 0 and distances silently become 0. Please reject meta.dimension() <= kTailBytes (and consider also rejecting dimensions that could make the stored tail semantics invalid for this metric).

  int init(const IndexMeta &meta, const ailego::Params &params) override {
    if (meta.data_type() != IndexMeta::DataType::DT_INT8) {
      LOG_ERROR("UniformUint8Metric: unsupported type %d", meta.data_type());
      return IndexError_Unsupported;
    }

src/core/quantizer/uniform_uint8_reformer.cc:123

revert() always interprets the payload as the shifted record layout (int8 storing uint8(code) - 128). However transform() explicitly emits the unshifted query layout (raw uint8 bytes stored in an int8 buffer). Calling revert() on query-encoded data will decode incorrectly (e.g., raw code 255 becomes 127). Consider either (a) documenting/enforcing that revert() only accepts record-encoded buffers, or (b) extending metadata to carry an explicit layout flag so revert() can decode both layouts safely.

  int revert(const void *input, const IndexQueryMeta &input_meta,
             std::string *output) const override {
    if (!initialized_ || !input || !output ||
        input_meta.data_type() != IndexMeta::DataType::DT_INT8) {
      return IndexError_InvalidArgument;
    }

    if (input_meta.dimension() <= kTailBytes) {
      return IndexError_InvalidArgument;
    }
    const size_t dimension = input_meta.dimension() - kTailBytes;
    output->resize(dimension * sizeof(float));
    auto *destination = reinterpret_cast<float *>(output->data());
    const auto *source = static_cast<const int8_t *>(input);
    const float inverse_scale = 1.0f / scale_;
    for (size_t i = 0; i < dimension; ++i) {
      const float raw_code = static_cast<float>(source[i]) + 128.0f;
      destination[i] = (raw_code - bias_) * inverse_scale;
    }
    return 0;
  }

src/core/quantizer/uniform_uint8_reformer.cc:202

The uint32 tail is computed by truncating sum_squared without a dimension bound check in the reformer. If transform()/convert() is ever called with dimension large enough, sum_squared can exceed uint32_t and silently wrap, breaking query scoring semantics. Consider validating dimension against the same bound used in the converter (or otherwise guaranteeing the tail range) before writing the tail.

    int64_t sum_squared = 0;
    for (size_t i = 0; i < dimension; ++i) {
      const int code = static_cast<int>(bytes[i]);
      sum_squared += code * code;
      if (shift_output) {
        bytes[i] = static_cast<uint8_t>(code - 128);
      }
    }
    const uint32_t tail = static_cast<uint32_t>(sum_squared);
    std::memcpy(output + dimension, &tail, sizeof(tail));

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/include/zvec/core/interface/index_param.h:1

This is a source-breaking public API change: kUniformInt8 is removed rather than renamed. If external users reference QuantizerType::kUniformInt8, they will no longer compile. Consider keeping a backwards-compatible alias (e.g., kUniformInt8 = kUniformUint7) and deprecating it, while still preserving the same numeric value.

// Copyright 2025-present the zvec project

src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.cc:326

When orig_dim == 0, the function returns without writing distances, leaving caller-provided buffers uninitialized. For consistency with the scalar distance path (which returns 0.0 for empty vectors), this should explicitly set distances[0..n) to 0.0f before returning.

  const size_t orig_dim = original_dim(dim);
  if (orig_dim == 0) {
    return;
  }

+  // Covers both sum(code^2) and the asymmetric query score for every
+  // dimension supported by the public API.
+  static_assert(uint64_t{MAX_DIMENSION} * 255 * 256 <=
+                (std::numeric_limits<uint32_t>::max)());


Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/core/interface/index_param.cc:403

This changes deserialization semantics: if the JSON object is missing the type field, the function now silently succeeds without setting type (previously DESERIALIZE_ENUM_FIELD likely enforced presence). To avoid accepting partially-specified configs, either (a) require type and return false when it’s absent, or (b) explicitly set a documented default value when missing. This keeps behavior predictable for callers and avoids accidental use of an unintended quantizer type.

  ailego::JsonValue type_json_value;
  if (json_obj.has("type")) {
    json_obj.get("type", &type_json_value);
    if (type_json_value.is_string() &&
        type_json_value.as_stl_string() == "kUniformInt8") {
      type = QuantizerType::kUniformUint7;
    } else if (!extract_enum_from_json<QuantizerType>(
                   json_obj, "type", type, type_json_value)) {
      LOG_ERROR("Error when deserialize json - field:type");
      return false;
    }
  }

src/turbo/distance/avx512_vnni/uniform_uint8/quantize.cc:29

get_uniform_uint8_quantize_func only returns this function when AVX512-VNNI is available, but the implementation is purely scalar. That can unintentionally degrade hot-path performance by adding an extra dispatch layer without SIMD benefit. Consider either implementing an actual AVX-512 vectorized quantize kernel here, or returning nullptr from get_uniform_uint8_quantize_func until a SIMD implementation exists (so callers reliably use their existing scalar path).

void uniform_uint8_quantize(const float *in, std::size_t dim, float scale,
                            float bias, std::int8_t *out) {
  auto *bytes = reinterpret_cast<std::uint8_t *>(out);
  for (std::size_t i = 0; i < dim; ++i) {
    float value = std::round(in[i] * scale + bias);
    value = std::max(0.0f, std::min(255.0f, value));
    bytes[i] = static_cast<std::uint8_t>(value);
  }
}

+#else
+  (void)vectors;
+  (void)query;
+#endif


Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/core/interface/index_param.cc:403

If the JSON object does not contain the type field, this function returns true without setting type, which can silently accept malformed configs and leave type at a default/previous value. Consider treating missing type as a hard error (log + return false), or explicitly defaulting in a controlled way to preserve the previous behavior of DESERIALIZE_ENUM_FIELD (which typically enforces required fields).

  ailego::JsonValue type_json_value;
  if (json_obj.has("type")) {
    json_obj.get("type", &type_json_value);
    if (type_json_value.is_string() &&
        type_json_value.as_stl_string() == "kUniformInt8") {
      type = QuantizerType::kUniformUint7;
    } else if (!extract_enum_from_json<QuantizerType>(
                   json_obj, "type", type, type_json_value)) {
      LOG_ERROR("Error when deserialize json - field:type");
      return false;
    }
  }

src/core/quantizer/uniform_uint8_reformer.cc:211

The squared-sum tail is truncated to uint32_t without an explicit guarantee in this translation unit that sum_squared always fits. Since correctness depends on MAX_DIMENSION and the max code value, consider adding the same static_assert bounds used in UniformUint8StreamingConverter (or a defensive runtime check) to keep the overflow assumption local and prevent future changes to MAX_DIMENSION from silently corrupting tails.

    int64_t sum_squared = 0;
    for (size_t i = 0; i < dimension; ++i) {
      const int code = static_cast<int>(bytes[i]);
      sum_squared += code * code;
      if (shift_output) {
        bytes[i] = static_cast<uint8_t>(code - 128);
      }
    }
    const uint32_t tail = static_cast<uint32_t>(sum_squared);
    std::memcpy(output + dimension, &tail, sizeof(tail));

+#else
+  (void)vectors;
+  (void)query;
+#endif
+}


Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/core/metric/uniform_uint8_metric.cc:166

The turbo batch-distance path for UniformUint8 uses an AVX512-VNNI dpbusd accumulator that is inherently int32-bounded for dot products. This metric implementation doesn’t enforce an upper bound on the encoded/original dimension before selecting the turbo kernel, so initializing the metric with an unusually large dimension can produce silent overflow and incorrect scores (even if scalar fallback would be correct). A concrete fix is to gate turbo dispatch on a safe maximum dimension (e.g., the same MAX_DIMENSION constraint used by the UniformUint8 converter/reformer), and otherwise fall back to UniformUint8StoredQueryScoreBatch (or implement a block-reduced SIMD path that avoids int32 overflow).

    auto turbo_distance = turbo::get_batch_distance_func(
        turbo::MetricType::kSquaredEuclidean, turbo::DataType::kInt8,
        turbo::QuantizeType::kUniformUint8);
    return turbo_distance ? turbo_distance : UniformUint8StoredQueryScoreBatch;
  }

Copilot

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/core/interface/index.cc:389

Index::Open now always recreates reformer_ from streamer meta when converter_ != nullptr, even if a reformer instance already exists from Init(). This can unnecessarily allocate and may discard any non-param state held by an existing reformer implementation. A more robust pattern is: if reformer_ already exists and matches meta.reformer_name(), call reformer_->init(meta.reformer_params()); otherwise create a new reformer and init it. This still handles deferred-params reformers while minimizing churn.

  // Recreate the reformer from the meta loaded by the streamer. Some
  // converters create a deferred reformer during Init(), before trained
  // parameters are available, so a non-null reformer_ does not mean it is
  // ready for search. When there is no converter (QuantizerType::kNone),
  // reformer_ is nullptr by design and this block is skipped.
  if (converter_ != nullptr) {
    const auto &meta = streamer_->meta();
    if (meta.reformer_name().empty()) {
      LOG_ERROR(
          "Index::Open: converter exists but streamer meta has no "
          "reformer");
      return core::IndexError_Runtime;
    }
    reformer_ = core::IndexFactory::CreateReformer(meta.reformer_name());

src/core/quantizer/uniform_uint8_reformer.cc:157

UniformUint8StreamingReformer returns IndexError_InvalidArgument when called before it has been initialized with trained params (!initialized_). Other reformers in this PR (e.g., UniformUint7) treat this as a runtime/uninitialized error. Consider returning IndexError_Runtime (or IndexError_Uninitialized if that’s the project convention) specifically for !initialized_, and reserving InvalidArgument for null pointers / count==0, to make failure modes clearer and consistent for API consumers.

    if (!initialized_ || !input || !output || !output_meta || count == 0) {
      return IndexError_InvalidArgument;
    }

+  const int64_t correction = static_cast<int64_t>(tail(query, orig_dim)) -
+                             256 * static_cast<int64_t>(sum);
+  if (correction < (std::numeric_limits<int32_t>::min)() ||
+      correction > (std::numeric_limits<int32_t>::max)()) {
+    // The public quantizer dimension bound keeps the correction in int32.
+    // Oversized direct calls use the scalar squared-difference fallback.
+    return;
+  }
+  const int32_t encoded_correction = static_cast<int32_t>(correction);
+  std::memcpy(static_cast<uint8_t *>(query) + orig_dim, &encoded_correction,
+              sizeof(encoded_correction));


+  const int64_t correction =
+      static_cast<int64_t>(Tail(query, original_dimension)) -
+      256 * static_cast<int64_t>(sum);
+  if (correction < (std::numeric_limits<int32_t>::min)() ||
+      correction > (std::numeric_limits<int32_t>::max)()) {
+    // Oversized externally supplied metadata uses the scalar squared-
+    // difference path, which ignores the query tail.
+    return;
+  }
+  const int32_t encoded_correction = static_cast<int32_t>(correction);
+  std::memcpy(static_cast<uint8_t *>(query) + original_dimension,
+              &encoded_correction, sizeof(encoded_correction));


Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/core/quantizer/uniform_uint8_reformer.cc:157

UniformUint8StreamingReformer treats the uninitialized/deferred state (!initialized_) as IndexError_InvalidArgument, while the canonical UniformUint7 reformer path returns a runtime-style error when scale/bias are missing (i.e., called before receiving trained params). Consider returning IndexError_Runtime (or the project’s dedicated uninitialized error, if available) when !initialized_, and reserving IndexError_InvalidArgument for actual bad inputs. This makes error handling consistent for callers across quantizers and better reflects the failure mode.

  int Encode(const void *input, const IndexQueryMeta &input_meta,
             uint32_t count, std::string *output, IndexQueryMeta *output_meta,
             bool shift_output) const {
    if (!initialized_ || !input || !output || !output_meta || count == 0) {
      return IndexError_InvalidArgument;
    }

tests/core/quantizer/uniform_uint8_reformer_test.cc:256

Grammatically, the test name should use the third-person singular: EmptyParamsResetsPreviouslyInitializedState.

TEST(UniformUint8Reformer, EmptyParamsResetPreviouslyInitializedState) {

 #include "avx512/rotate/fht/fht.h"
 #include "avx512_vnni/record_quantized_int8/cosine.h"
 #include "avx512_vnni/record_quantized_int8/squared_euclidean.h"
-#include "avx512_vnni/uniform_int8/quantize.h"
-#include "avx512_vnni/uniform_int8/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


Copilot

Pull request overview

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

 #include "avx512/rotate/fht/fht.h"
 #include "avx512_vnni/record_quantized_int8/cosine.h"
 #include "avx512_vnni/record_quantized_int8/squared_euclidean.h"
-#include "avx512_vnni/uniform_int8/quantize.h"
-#include "avx512_vnni/uniform_int8/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/include/zvec/core/interface/index_param.h:1

Removing/renaming kUniformInt8 to kUniformUint7 is likely a breaking change for any persisted configs/JSON that still reference "kUniformInt8" (even though the numeric value is preserved). If backward compatibility matters, consider keeping a deprecated alias enumerator (e.g., kUniformInt8 = kUniformUint7) and/or accepting "kUniformInt8" during JSON deserialization as a synonym for kUniformUint7.

// Copyright 2025-present the zvec project

+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

tests/core/quantizer/uniform_uint8_reformer_test.cc:310

The expected value is uint64_t while ReadTail(...) returns uint32_t. Even though the value is in-range (per the surrounding static_asserts), using mismatched integer widths can introduce compiler warnings and can make gtest diagnostics less clear. Prefer casting the expected value to uint32_t (or change ReadTail to return uint64_t in the test helpers) so the comparison is type-consistent.

  EXPECT_EQ(kMaxDimension + sizeof(uint32_t), output_meta.dimension());
  EXPECT_EQ(uint64_t{kMaxDimension} * 255 * 255,
            ReadTail(output.data(), kMaxDimension));

 #include "avx512/rotate/fht/fht.h"
 #include "avx512_vnni/record_quantized_int8/cosine.h"
 #include "avx512_vnni/record_quantized_int8/squared_euclidean.h"
-#include "avx512_vnni/uniform_int8/quantize.h"
-#include "avx512_vnni/uniform_int8/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


Copilot

Pull request overview

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

+    if (!initialized_ || !input || !output ||
+        record_meta.data_type() != IndexMeta::DataType::DT_INT8) {
+      return IndexError_InvalidArgument;
+    }
+
+    if (record_meta.dimension() <= kTailBytes) {
+      return IndexError_InvalidArgument;
+    }
+    const size_t dimension = record_meta.dimension() - kTailBytes;


Validate UniformUint7 dimensions and reformer inputs, and cover UniformUint8 graph and brute-force search through the Vamana query metric path.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

src/include/zvec/core/interface/index_param.h:101

Replacing/removing the prior kUniformInt8 enum value is a breaking API change for any persisted configs/JSON that still specify kUniformInt8 (and potentially for any integer-based serialization if used elsewhere). To preserve backward compatibility, consider keeping a deprecated alias (e.g., kUniformInt8 = kUniformUint7) and/or teaching QuantizerParam::DeserializeFromJson to accept legacy strings ("kUniformInt8") by mapping them to kUniformUint7.

  kInt8,
  kInt4,
  kRabitq,
  // Global uniform quantization with a shared scale/bias and 128 code values
  // in [0, 127].
  kUniformUint7 = 8,
  // Global uniform quantization with the full uint8 code range [0, 255].
  kUniformUint8 = 9,

src/core/quantizer/uniform_uint8_reformer.cc:201

UniformUint7 was updated to use std::nearbyint explicitly to match the SIMD kernel’s rounding behavior under the default rounding mode, but UniformUint8 still uses std::round (half away from zero). If UniformUint8 later gains a SIMD quantize path (or if you want consistent rounding semantics across uniform quantizers), prefer aligning this to the same rounding primitive/contract (e.g., nearbyint with a documented rounding-mode assumption) and add a test for halfway (.5) values to lock the behavior.

      float value = std::round(input[i] * scale_ + bias_);
      value = std::max(0.0f, std::min(255.0f, value));
      bytes[i] = static_cast<uint8_t>(value);

+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


Copilot

Pull request overview

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

Comments suppressed due to low confidence (5)

src/include/zvec/core/interface/index_param.h:101

This change removes QuantizerType::kUniformInt8 in favor of kUniformUint7/kUniformUint8, which is a breaking API/serialization change for any external configs or persisted metadata that refer to the old enum value/name. If backward compatibility is required, consider keeping a deprecated alias (e.g., kUniformInt8 = kUniformUint7) and/or mapping legacy serialized strings to the new type during JSON deserialization.

  // Global uniform quantization with a shared scale/bias and 128 code values
  // in [0, 127].
  kUniformUint7 = 8,
  // Global uniform quantization with the full uint8 code range [0, 255].
  kUniformUint8 = 9,

tests/core/metric/uniform_uint7_metric_test.cc:30

This test hard-codes the param key string instead of using the shared constant (UNIFORM_UINT7_METRIC_ORIGIN_METRIC_NAME) that was added in src/core/metric/metric_params.h. Using the constant would prevent silent drift if the key changes and aligns with how the UniformUint8 metric tests reference metric_params.h.

  ailego::Params params;
  params.set("proxima.uniform_uint7.metric.origin_metric_name",
             std::string("SquaredEuclidean"));

tests/core/metric/uniform_uint7_metric_test.cc:37

This test hard-codes the param key string instead of using the shared constant (UNIFORM_UINT7_METRIC_ORIGIN_METRIC_NAME) that was added in src/core/metric/metric_params.h. Using the constant would prevent silent drift if the key changes and aligns with how the UniformUint8 metric tests reference metric_params.h.

  std::string metric_name;
  EXPECT_TRUE(metric->params().get(
      "proxima.uniform_uint7.metric.origin_metric_name", &metric_name));

tests/core/metric/uniform_uint8_metric_test.cc:368

Including very large dimensions (e.g., 1,048,577) in a unit test can significantly increase CI runtime and memory pressure, especially since the test also computes a scalar reference (SquaredL2) and encodes buffers of size O(dimension). Consider reducing the maximum dimension for the default test suite, or gating the largest case behind a slow/stress test flag (or GTEST_SKIP() unless an explicit env var is set).

TEST(UniformUint8Metric, BuildDistanceIsExactForFullRangeAndLargeDimensions) {
  std::mt19937 generator(20260716);
  std::uniform_int_distribution<int> byte_distribution(0, 255);

  for (const size_t dimension : {1UL, 31UL, 32UL, 33UL, 127UL, 128UL, 129UL,
                                 1024UL, 65536UL, 1048577UL}) {

src/core/quantizer/uniform_uint8_reformer.cc:201

UniformUint7 quantization was updated to use std::nearbyint to match SIMD rounding behavior (nearest-even under default rounding mode), but UniformUint8 still uses std::round (half away from zero). If the intended contract is consistent rounding behavior across uniform quantizers (and to avoid systematic bias around half steps), consider switching this to std::nearbyint or explicitly documenting why uint8 differs.

      float value = std::round(input[i] * scale_ + bias_);
      value = std::max(0.0f, std::min(255.0f, value));
      bytes[i] = static_cast<uint8_t>(value);

+#include "avx512_vnni/uniform_uint7/quantize.h"
+#include "avx512_vnni/uniform_uint7/squared_euclidean.h"
+#include "avx512_vnni/uniform_uint8/squared_euclidean.h"


luoxiaojian requested review from Cuiyus, chinaux, iaojnh, richyreachy and zhourrr as code owners July 20, 2026 09:05

github-actions Bot assigned luoxiaojian Jul 20, 2026

luoxiaojian force-pushed the codex/uniform-uint7-uint8 branch 4 times, most recently from da35854 to 6df5dd8 Compare July 20, 2026 11:52

luoxiaojian force-pushed the codex/uniform-uint7-uint8 branch from 21ad9db to 332bda5 Compare July 29, 2026 08:04

Copilot AI review requested due to automatic review settings July 29, 2026 08:04

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 09:41 View session

Copilot AI review requested due to automatic review settings July 29, 2026 10:51

luoxiaojian force-pushed the codex/uniform-uint7-uint8 branch from 3b1adc8 to b8caffc Compare July 29, 2026 11:01

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 11:10

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 29, 2026 11:31

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Comment thread src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.cc

Comment on lines +349 to +352

#else

(void)vectors;

(void)query;

#endif

Copilot AI review requested due to automatic review settings July 29, 2026 11:49

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Comment thread src/turbo/distance/avx512_vnni/uniform_uint8/squared_euclidean.cc

Comment on lines +349 to +353

#else

(void)vectors;

(void)query;

#endif

}

Copilot AI review requested due to automatic review settings July 29, 2026 12:15

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 12:24 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 12:57 View session

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 13:00 View session

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 13:16 View session

Copilot started reviewing on behalf of luoxiaojian July 29, 2026 13:26 View session

Copilot AI review requested due to automatic review settings July 30, 2026 10:00

Copilot started reviewing on behalf of luoxiaojian July 30, 2026 10:05 View session