shyjsarah commented on PR #82: URL: https://github.com/apache/paimon-vector-index/pull/82#issuecomment-5504114955
Re-reviewed the latest head (`9e6cb1b`). CI is green, the core suite passes locally (471 passed / 1 ignored), and the previous no-AVX2/FMA small-add transpose regression is fixed by restoring the direct path. The new pairwise-margin certificate fixes the earlier two-code fixture, but I found a remaining correctness hole plus two search-performance blockers: 1. **The certificate proves only the global table argmin, while IVF-PQ scans arbitrary persisted codes.** Adding an unused third centroid that is a stable global argmin suppresses exact fallback, while stored codes 0 and 1 remain reversed. On the latest head, add emits `[0,1]` and direct/FMA scoring ranks id 1 first, but ordinary in-memory, reopened reader, forced ephemeral reuse, resident optimized reader, and in-memory precomputed search all return id 0. 2. **The certificate broadly regresses standard-scale L2 search.** Compared with `7296eaa`, a short standard-scale benchmark measured ordinary search about +36%, resident precomputed about +56%, and ephemeral reuse about +217%. Each completed 256-entry sub-table is rescanned by `min_by` and `all()`, with repeated error-bound work in the hot table-building loops. 3. **Unstable ephemeral fallback pays both algorithms and allocates per query/list.** Large-offset Auto ephemeral search measured about `6.6 ms` versus `0.46 ms` at `7296eaa` (~14x). The fallback makes exactly `nq*nprobe` extra residual-vector allocations and recomputes the complete exact table after ephemeral list/query tables were already built. Suggested direction: - Do not use stability of the global centroid argmin as a proxy for the ordering of arbitrary stored codes. Either use the canonical direct/FMA table or certify the comparisons actually consumed by scanning. - Fuse stability tracking into table construction instead of rescanning completed tables. - Reuse a per-worker residual buffer and avoid selecting ephemeral reuse when the scale predicts a high exact-fallback rate. Minor follow-up: the small-batch dispatch policy would be more robust with an explicit kernel-kind enum rather than function-address equality. I still recommend holding the merge until the three blockers above are addressed. -- 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]
