shyjsarah commented on PR #82: URL: https://github.com/apache/paimon-vector-index/pull/82#issuecomment-5451935967
Re-reviewed the latest head (`2f0ffa9`). The earlier subnormal-correctness and small-`nprobe` serialization issues appear fixed, but two blockers remain: 1. **Scalar high-`nprobe` regression is still unresolved.** `find_topk` selects the bounded-heap path using `rayon::current_num_threads()`, even though a single scalar query is not parallelized internally. On the latest head, for `d=8, k=262144, nprobe=65536`, I measured a median of **2.51 ms** in a 1-thread pool versus **11.35 ms** in an 8-thread pool (~4.5x slower). The algorithm choice should not depend on ambient pool width for a scalar call; it should reflect actual concurrent queries/scratch usage, or use the linear-selection path here. 2. **CI is currently red.** Besides the formatting failure, `test_mixed_block_sgemm_only_processes_safe_rows` now gets `gemm_rows=0` instead of `16`. The test uses `d=1`, so the new `MIN_GEMM_DIM=32` cutoff routes it through the direct path and the test no longer exercises the mixed SGEMM branch it is intended to verify. Please update the fixture to use `d >= 32` while retaining mixed safe/unsafe rows. So this is improved, but I don't think the PR is ready to merge yet. -- 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]
