feat(benchmark): add Annoy migration benchmarks - #1958
Conversation
379bece to
e8945ae
Compare
8cc85b0 to
a1ef2c7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Replace the default BasicEmbeddingsIndex implementation with exact cosine search over a normalized NumPy matrix while preserving Annoy angular-distance scores so existing thresholds keep their behavior. Keep default knowledge-base index persistence in the BasicEmbeddingsIndex backend and store cached indexes as .npy files, removing the KB layer's direct Annoy dependency without expanding the base EmbeddingsIndex contract. Drop Annoy from the project dependencies and lockfile, and cover the NumPy index ranking, threshold parity, and save/load round-trips.
e8945ae to
a11f8cd
Compare
a1ef2c7 to
2feaa4e
Compare
Greptile SummaryThis PR adds a standalone benchmark harness (
|
| Filename | Overview |
|---|---|
| benchmark/embedding_backend/bench_embedding_backend.py | New benchmark harness for Annoy vs NumPy; two minor issues: mean_ms stored in result rows but never rendered in the markdown table, and exact_topk silently returns fewer than k columns when n < k causing misleading recall labels. |
| benchmark/embedding_backend/README.md | New documentation for the benchmark; methodology, reproduction steps, and captured results are clearly explained. The mem (MB) = 0.0 results are properly footnoted as platform-specific (non-Linux). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[main] --> B[parse args]
B --> C{annoy_available?}
C -- yes --> D[BACKENDS = annoy + numpy]
C -- no --> E[BACKENDS = numpy only]
D --> F[run]
E --> F
F --> G[for each N in sizes]
G --> H[make_normalized_vectors matrix]
H --> I[make_normalized_vectors queries]
I --> J[exact_topk ground truth]
J --> K[for each backend]
K --> L[builder → build_s, search_fn, mem_mb]
L --> M[warm-up call]
M --> N[time_searches → results, p50, p95, mean]
N --> O[recall_at_k × 2]
O --> P[append row dict]
P --> K
K -- next backend --> K
G -- next N --> G
F --> Q[print_markdown]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
benchmark/embedding_backend/bench_embedding_backend.py:197
`mean_ms` — dead dict field
`mean` is captured from `time_searches` and stored in every result row as `mean_ms`, but `print_markdown` never reads it and the progress `print()` on line 203–207 doesn't print it either. This is the same pattern as the recently removed `rec5` field. Either render it in the markdown table (as an additional `search mean (ms)` column) or drop the field and the fourth return value from `time_searches` to keep the output and the stored data consistent.
### Issue 2 of 2
benchmark/embedding_backend/bench_embedding_backend.py:80-87
`exact_topk` silently truncates ground truth when `n < k`
The docstring promises shape `(Q, k)`, but when `n < k` the `argpartition` result has only `n` columns, and NumPy's permissive slicing means `[:, :k]` silently returns `(Q, n)` instead of raising. Downstream, `recall_at_k(results, truth, k)` then treats `n` items as the full ground-truth set, making both backends score 1.0 trivially. The markdown table would label the column `recall@20` while actually reporting something different. This doesn't trigger with the default sizes (≥100) and k=20, but a user passing e.g. `--sizes 5 10 --k 20` would get silently wrong recall numbers. A guard or assertion in `run()` would prevent the confusion.
Reviews (5): Last reviewed commit: "apply review suggestions" | Re-trigger Greptile
Remove compiler flags, cache-key suffixes, Docker environment variables, and setup docs that were only needed for the Annoy native extension. Update installation and embedding-provider docs so the default install reflects the NumPy-backed search implementation.
2feaa4e to
6b34498
Compare
Keep gcc and g++ in the Docker images for optional and dev dependencies that may need native builds, while leaving the Annoy-specific compiler flags removed.
6b34498 to
40fd78a
Compare
Install only the server extra in the runtime Docker image and remove the redundant second install so the Docker CI build avoids pulling every optional and dev dependency. Keep the QA Dockerfile on dev dependencies while avoiding all optional extras and the duplicate install.
40fd78a to
e10e0f7
Compare
e10e0f7 to
6639b54
Compare
83ab3b4 to
6bae105
Compare
6bae105 to
c4ac05e
Compare
tgasser-nv
left a comment
There was a problem hiding this comment.
This is great, thanks for doing the benchmarking to quantify the tradeoffs between annoy and numpy. I ran a regression on the Results table and the breakeven point is N ~= 5,000. I can't imagine anyone having more than 100 dialog rules, so this is well underneath the breakeven point.
Add a reproducible benchmark harness for comparing the previous Annoy backend with the exact NumPy backend across build time, search latency, recall, and memory. Document benchmark methodology, captured baseline results, and reproduction steps while keeping Annoy as an optional benchmark-only install.
c4ac05e to
b05aa97
Compare
Summary
Adds benchmark tooling and reproduction notes used to compare the previous Annoy backend with the new exact NumPy backend.
What Changed
Stack
Part 2 of 2.
drop-annoy-featuredrop-annoy-benchmarkReview this PR against
drop-annoy-feature; the implementation change is in the parent PR.Validation