Sq8 to Sq8 dist functions - ip and cosine [MOD-13170] #873
Conversation
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
- Implemented inner product and cosine distance functions for SQ8-to-SQ8 vectors in SVE, NEON, and AVX512 architectures. - Added corresponding distance function selection logic in IP_space.cpp and function headers in IP_space.h. - Created benchmarks for SQ8-to-SQ8 distance functions to evaluate performance across different architectures. - Developed unit tests to validate the correctness of the new distance functions against expected results. - Ensured compatibility with existing optimization features for various CPU architectures.
dor-forer
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for SQ8-to-SQ8 distance functions for Inner Product and Cosine similarity metrics, where both vectors are scalar-quantized 8-bit representations. This extends the existing SQ8 functionality which only handled float-to-SQ8 comparisons.
Key changes:
- Implemented optimized SIMD versions for Intel (AVX512) and ARM (SVE, SVE2, NEON) architectures
- Added comprehensive test coverage for all optimization variants
- Integrated new benchmark suite for performance testing
- Refactored existing helper function names to avoid naming conflicts
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_spaces.cpp | Added unit tests for SQ8_SQ8 IP and Cosine functions across all architectures; improved test suite naming consistency |
| tests/benchmark/spaces_benchmarks/bm_spaces_sq8_sq8.cpp | New benchmark file for SQ8_SQ8 distance functions |
| tests/benchmark/spaces_benchmarks/bm_spaces_sq8.cpp | Removed empty comment lines (cleanup) |
| tests/benchmark/benchmarks.sh | Added spaces_sq8_sq8 to benchmark configurations |
| tests/benchmark/CMakeLists.txt | Added sq8_sq8 data type to build system |
| src/VecSim/spaces/functions/SVE2.h | Added function declarations for SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/functions/SVE2.cpp | Implemented SVE2 function choosers for SQ8_SQ8 distance functions |
| src/VecSim/spaces/functions/SVE.h | Added function declarations for SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/functions/SVE.cpp | Implemented SVE function choosers for SQ8_SQ8 distance functions |
| src/VecSim/spaces/functions/NEON.h | Added function declarations for SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/functions/NEON.cpp | Implemented NEON function choosers for SQ8_SQ8 distance functions |
| src/VecSim/spaces/functions/AVX512F_BW_VL_VNNI.h | Added function declarations for SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/functions/AVX512F_BW_VL_VNNI.cpp | Implemented AVX512 function choosers for SQ8_SQ8 distance functions |
| src/VecSim/spaces/IP_space.h | Added public API declarations for SQ8_SQ8 distance functions |
| src/VecSim/spaces/IP_space.cpp | Implemented dispatcher functions that select appropriate SIMD implementation |
| src/VecSim/spaces/IP/IP_SVE_SQ8_SQ8.h | New file with SVE-optimized SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/IP/IP_NEON_SQ8_SQ8.h | New file with NEON-optimized SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/IP/IP_AVX512F_SQ8_SQ8_BW_VL_VNNI.h | New file with AVX512-optimized SQ8_SQ8 IP and Cosine implementations |
| src/VecSim/spaces/IP/IP_AVX512F_SQ8_BW_VL_VNNI.h | Renamed helper function to avoid naming conflicts with new SQ8_SQ8 implementations |
| src/VecSim/spaces/IP/IP_AVX2_SQ8.h | Renamed helper function to avoid naming conflicts and fixed cosine normalization |
| src/VecSim/spaces/IP/IP.h | Added function declarations for SQ8_SQ8 base implementations |
| src/VecSim/spaces/IP/IP.cpp | Implemented baseline SQ8_SQ8 InnerProduct and Cosine functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #873 +/- ##
==========================================
+ Coverage 97.06% 97.09% +0.02%
==========================================
Files 126 127 +1
Lines 7403 7574 +171
==========================================
+ Hits 7186 7354 +168
- Misses 217 220 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… SQ8-to-SQ8 calculations
… NEON and AVX512 headers
…ocumentation accordingly
…tance assertion tolerance
… using AVX512 VNNI; add benchmarks and tests for new functionality
…VE, and AVX512; add corresponding selection functions and update tests for consistency.
…update benchmarks and tests for new functionality
- Updated distance function declarations in IP_space.h to clarify that SQ8-to-SQ8 functions use precomputed sum/norm. - Removed precomputed distance function implementations for AVX512F, NEON, and SVE architectures from their respective source files. - Adjusted benchmark tests to remove references to precomputed distance functions and ensure they utilize the updated quantization methods. - Modified utility functions to support the creation of SQ8 quantized vectors with precomputed sum and norm. - Updated unit tests to reflect changes in the quantization process and removed tests specifically for precomputed distance functions.
…nsistency - Updated include paths in AVX512F_BW_VL_VNNI.cpp to reflect new naming conventions. - Modified unit tests in test_spaces.cpp to streamline vector initialization and quantization processes. - Replaced repetitive code with utility functions for populating and quantizing vectors. - Enhanced assertions in tests to ensure optimized distance functions are correctly chosen and validated. - Removed unnecessary parameters from utility functions to simplify their interfaces. - Improved test coverage for edge cases, including zero and constant vectors, ensuring accuracy across various scenarios.
… vector population methods
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe the changes in the pull request
Added support to SQ8 to SQ8 Ip and Cosine spaces.
Intel:
AVX512_F_BW_VL_VNNI - needs it to support uint8 and in8 operations.
ARM:
SVE
NEON
The cosine functions assume that the vectors are normlized therefore don't divide by the norm.
Which issues this PR fixes
Main objects this PR modified
Mark if applicable