shyjsarah commented on PR #82: URL: https://github.com/apache/paimon-vector-index/pull/82#issuecomment-5487883958
Re-reviewed the latest head (`314c6bf`). The implementation direction is good, but I found three blockers, so I don't think this is ready to merge yet: 1. **Add-time encoding and search-time scoring use inconsistent L2 arithmetic.** The new blocked encoder minimizes direct squared differences, while IVF-PQ search still uses the cancellation-prone `||q||² + ||c||² - 2q·c` table. I reproduced this through the public train/add/write/open/search APIs using finite data: an exact indexed query row was scored at `8388608`, while a farther row was scored at `0` and returned first. The merge base did not show this reversal. 2. **The `<32` direct path regresses x86_64 small-batch adds.** For the included 31-row `d=768,m=192` shape, an independent optimized x86_64 run measured about `3.63 ms` on this PR versus `1.11 ms` at the merge base (>3x slower). The generic `f32::mul_add` loop compiles to inner-loop `fmaf` calls on x86_64. 3. **`dsub>4` also regresses on x86_64.** The transposed path is enabled for every `dsub>=4`, but SIMD specialization only handles `dsub==4`; the documented 32,768-row `dsub=8` case measured about `212 ms` on this PR versus `74 ms` at the merge base (~3x slower). Suggested direction: - Use one numerically consistent distance definition for both PQ assignment and all IVF-PQ search-table paths, with an end-to-end large-offset regression test. - Keep the previous x86 path for small batches and unsupported `dsub` shapes until suitable AVX2 kernels are available, or introduce target-specific thresholds/kernels. - Follow-up: add required aarch64 core-test coverage for the new unsafe NEON implementation. Local verification otherwise passed: core tests (465 passed, 1 ignored), targeted PQ tests, workspace Clippy, fmt check, and benchmark compilation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
