[codex] feat: add dense SignBitmap scoring#96
Conversation
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces dense full-corpus sign-agreement scoring methods (score_all and score_all_batched) to the SignBitmap struct, exposing them to Python via PyO3. The feedback focuses on a significant performance optimization: refactoring score_all_batched to return a flat Vec<u32> instead of a nested Vec<Vec<u32>>. This change avoids redundant heap allocations and allows the Python FFI layer to construct the 2D NumPy array efficiently. Corresponding updates to the Python bindings and unit tests are also provided to support this optimization.
Review Summary by QodoAdd dense SignBitmap scoring methods for full-corpus evaluation
WalkthroughsDescription• Add score_all() method for dense full-corpus sign-agreement scoring • Add score_all_batched() for batched dense scoring without top-k selection • Expose both methods to Python with proper NumPy array handling • Add comprehensive tests for correctness and edge cases Diagramflowchart LR
A["Query Vector"] -->|"score_all()"| B["Dense Scores"]
C["Query Batch"] -->|"score_all_batched()"| D["Dense Score Matrix"]
B -->|"NumPy uint32"| E["Python API"]
D -->|"NumPy uint32"| E
F["Hamming Distance"] -->|"Convert to Agreement"| B
F -->|"Convert to Agreement"| D
File Changes1. src/sign_bitmap.rs
|
Code Review by Qodo
1.
|
There was a problem hiding this comment.
Pull request overview
Adds full-corpus (“dense”) sign-agreement scoring APIs to SignBitmap in Rust and exposes them in the Python bindings to support QPP-style distribution statistics (issue #79).
Changes:
- Add
SignBitmap::score_allandSignBitmap::score_all_batchedto return dense per-doc sign-agreement counts (no top-k, no sorting). - Expose
SignBitmap.score_all(...)/score_all_batched(...)in Python asuint32NumPy arrays and extend the Python docstring to mention dense scoring methods release the GIL. - Add Rust + Python tests to validate shape/dtype and reference parity for single and batched scoring, including empty-shape behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/sign_bitmap.rs |
Implements dense per-document scoring APIs and adds Rust unit tests for parity and empty cases. |
ordvec-python/src/lib.rs |
Adds PyO3 bindings for dense scoring methods and returns results as NumPy uint32 arrays. |
ordvec-python/tests/test_sign_bitmap.py |
Adds Python tests for shape/dtype and correctness against a NumPy reference implementation. |
ordvec-python/python/ordvec/__init__.py |
Updates module docstring to include dense scoring methods in the GIL-release/threading contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
|
Bot review disposition after follow-up fix:
Validation run locally:
|
Summary
SignBitmap::score_all(&[f32]) -> Vec<u32>returning dense sign-agreement counts aligned by doc idSignBitmap::score_all_batched(&[f32]) -> Vec<Vec<u32>>for query batches without top-k selection or sortingSignBitmap.score_all(query)andSignBitmap.score_all_batched(queries)as denseuint32NumPy arraysCloses #79.
Validation
cargo fmt --all --checkcargo test -p ordvec sign_bitmap::tests::score_allcargo clippy --all-targets --all-features -- -D warningscargo test --all-featuresmaturin develop./.venv/bin/python -m pytest tests/test_sign_bitmap.py -q./.venv/bin/python -m pytest tests -q