Skip to content

Quantization improvements#87

Closed
glookka wants to merge 68 commits into
masterfrom
quantization_improvements
Closed

Quantization improvements#87
glookka wants to merge 68 commits into
masterfrom
quantization_improvements

Conversation

@glookka

@glookka glookka commented May 14, 2025

Copy link
Copy Markdown
Contributor

Quantization improvements

@glookka glookka requested a review from Copilot May 14, 2025 11:02

Copilot AI left a comment

Copy link
Copy Markdown

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 refactors quantization functionality and updates several APIs to support improved quantization settings and binary space representations. Key changes include:

  • New quantization-related functions and API modifications in util_private and knn modules.
  • Updates to file mapping (reader) methods to support write modes.
  • Adjustments to index building and distance calculation interfaces for improved quantization support.

Reviewed Changes

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

Show a summary per file
File Description
util/util_private.h Added CalcNorm and templated vector distance functions; updated NormalizeVec signature.
util/util_private.cpp Updated NormalizeVec implementation to use division factor and return norm.
util/util.h Introduced a new resize function implementation using memset.
util/reader.{h,cpp} Modified file open methods to support write access by adding a bWrite parameter.
secondary/builder.cpp Updated call to Open with new bWrite parameter.
knn/space.h Modified SetQuantizationSettings signature and updated data size calculations.
knn/quantizer.h Revised Encode signature to include error handling and added GetPoolFetcher.
knn/knn.{h,cpp} Updated quantization settings I/O and modified distance/index building APIs accordingly.

Comment thread util/util.h Outdated
Comment thread knn/space.h

protected:
float CalcAlpha ( const QuantizationSettings_t & tSettings ) const override { return ( tSettings.m_fMax-tSettings.m_fMin ) / 15.0; }
size_t get_data_size() override { return ( (m_uDim+3)>>1 ) + sizeof(float); }

Copilot AI May 14, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The modified get_data_size() now adds sizeof(float) to the computed size; please ensure this addition is thoroughly documented and intentional since it mixes element count with a size in bytes.

Suggested change
size_t get_data_size() override { return ( (m_uDim+3)>>1 ) + sizeof(float); }
size_t get_data_size() override {
// The addition of sizeof(float) accounts for extra space required for metadata or alignment.
// This ensures compatibility with the internal data representation.
return ( (m_uDim+3)>>1 ) + sizeof(float);
}

Copilot uses AI. Check for mistakes.
glookka and others added 3 commits May 14, 2025 13:07
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented May 14, 2025

Copy link
Copy Markdown

Linux debug test results

  8 files    8 suites   11m 20s ⏱️
486 tests 469 ✅ 17 💤 0 ❌
500 runs  483 ✅ 17 💤 0 ❌

Results for commit e70a5a5.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented May 14, 2025

Copy link
Copy Markdown

Linux release test results

  8 files    8 suites   5m 57s ⏱️
486 tests 469 ✅ 17 💤 0 ❌
500 runs  483 ✅ 17 💤 0 ❌

Results for commit e70a5a5.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented May 14, 2025

Copy link
Copy Markdown

Windows test results

  5 files    5 suites   15m 39s ⏱️
469 tests 452 ✅ 17 💤 0 ❌
477 runs  460 ✅ 17 💤 0 ❌

Results for commit e70a5a5.

♻️ This comment has been updated with latest results.

@CLAassistant

CLAassistant commented Jun 12, 2025

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ sanikolaev
✅ glookka
❌ klirichek
You have signed the CLA already but the status is still pending? Let us recheck it.

glookka and others added 22 commits June 12, 2025 17:11
also use special cache for x86-64-v3
configures and builds columnar for x86-64-v3 arch (avx2), and adds built artefacts to package/debugsymbols.
on clang, _mm_popcnt_u32 expanded to __builtin_popcount,
that makes PopCnt32 useless (both branches are the same).
Also, 'FORCE_INLINE' doesn't work by unknown reason.

Build fails on 'unity' flavour, when about all files 'glued' together,
and we have finally two similar definitions in unified file.
we don't need second copy from external project
streamvbyte and hnsw was't build on our current cmake
This update introduces a step in the GitHub Actions workflows to determine the branch name for caching purposes. The branch name is set based on whether the event is a pull request or a regular push, allowing for more granular cache keys in the build and test workflows. This change is applied across build_template.yml, test_template.yml, and win_test_template.yml.
This update introduces print statements in the GitHub Actions workflows to display the value of the MANTICORE_LOCATOR input. It provides feedback on whether a custom locator is set or if the default sources will be used, enhancing visibility during the build and test processes. Changes are applied across build_template.yml, test_template.yml, and win_test_template.yml.
This update adds a step to determine the branch name for caching in the Windows workflow. The cache key is dynamically set based on whether the event is a pull request or a regular push.
@glookka glookka closed this Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants