Add core foundation modules#1
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for putting the initial core modules together. I found a blocker: the current PR does not compile.
Local validation on this head (35259c9741b1a27ee31a2c922a48dc96267e654e):
cargo test --all --all-features
fails in core/src/distance.rs because the AVX2 gather index expressions mix usize and u32, for example around lines 383-390:
7 * ksub + codes[i + 7] as u32
Here ksub is usize, while codes[...] as u32 makes the RHS u32, so Rust cannot add them. The same issue repeats for the 8 gather indices.
Please keep the index arithmetic in one integer type before casting to the intrinsic index type, e.g. use codes[...] as usize and cast the final result to i32 (or otherwise explicitly convert both operands safely). After that, please run the full Rust validation again.
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for the update. The previous compile blocker is fixed: cargo test --all --all-features now passes locally with 15 unit tests.
However, this PR still fails the formatter check that the CI workflow is going to enforce:
cargo fmt --all -- --check
It reports formatting diffs in core/src/blas.rs, core/src/distance.rs, core/src/kmeans.rs, and core/src/pq.rs.
Please run cargo fmt --all and push the formatted result. After that I can re-check and approve if nothing else shows up.
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for the formatting update. The previous formatter blocker is fixed now:
cargo fmt --all -- --check passes.
cargo test --all --all-features also still passes locally with 15 tests.
There is still one CI blocker: the same clippy command used by PR #2 fails on this branch:
cargo clippy --all-targets --workspace -- -D warnings
Current failures include clippy::too_many_arguments in core/src/blas.rs, multiple clippy::needless_range_loop findings in distance.rs, kmeans.rs, and pq.rs, clippy::manual_clamp, clippy::identity_op / clippy::erasing_op in the AVX2 gather offsets, clippy::manual_is_multiple_of, and clippy::bad_bit_mask in the 4-bit PQ assertions.
Since the CI workflow enforces -D warnings, please either fix these lints or add narrow #[allow(...)] annotations where the current style is intentional/performance-sensitive. After clippy passes, I can re-check and approve.
Placeholder crate with empty lib.rs — real code comes via PR apache#1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bring over the basic building blocks: distance (L2/IP/Cosine + SIMD), blas (sgemm), kmeans (k-means++/hierarchical/streaming), and product quantizer (train/encode/decode/distance table). Skips higher-level modules (ivfpq, opq, fastscan, shuffler, io) and language bindings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cast codes[] to usize instead of u32 so the addition with ksub (usize) compiles without type errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e1d8df4 to
42a8c91
Compare
- Allow needless_range_loop and too_many_arguments at crate level (appropriate for numerics/SIMD code) - Replace 0*ksub and 1*ksub with direct expressions - Use .clamp() instead of .min().max() - Use .is_multiple_of() instead of % == 0 - Remove trivially-true bit mask assertions in tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
leaves12138
left a comment
There was a problem hiding this comment.
Thanks for the update. I re-checked the latest head (4985aa0ac475800c1422f233b72b390a4874a5b2) after the clippy fixes.
Local validation now passes:
cargo fmt --all -- --checkcargo clippy --all-targets --workspace -- -D warningscargo build --all-targetscargo test --all --all-features(15 passed)
The GitHub CI checks for this PR are also green (check and rust-test). No further blocker from my side.
Summary
Test plan
cargo buildpassescargo testpasses (unit tests included in each module)