fix(retriever): avoid re-normalizing full in-memory matrix - #85
UdayKumar9381 wants to merge 5 commits into
Conversation
adaumsilva
left a comment
There was a problem hiding this comment.
@UdayKumar9381 Thanks for the optimization! Tests and CI pass, and retrieval ordering remains correct.
One issue needs fixing before merging: after concatenation in retrieve(), _matrix_blocks still retains every original array alongside the new full matrix. This doubles persistent NumPy vector storage.
Please consolidate the blocks so they reference the cached matrix after concatenation, and reuse the existing array when there is only one block. Add regression coverage confirming that consolidation avoids duplicate storage and that subsequent add/retrieve cycles preserve all results.
Please request another review once these changes are pushed and CI passes.
…ation' into fix/in-memory-retriever-normalization
|
Thanks for the review. I’ve addressed the requested changes: Consolidated _matrix_blocks after retrieval to avoid duplicate NumPy storage. The changes have been pushed to the PR. Please take another look when convenient. |
Description
Closes #27.
This PR optimizes
InMemoryRetriever.add()so that incremental ingestion does not rebuild and re-normalize the entire stored vector matrix on every call.Changes
CHANGELOG.mdunder[Unreleased] -> ### Fixed.Validation
pytest tests/test_retriever/test_in_memory.py -q— 33 passedpytest -q— 266 passedruff check ragframework/ tests/— passedpre-commit run --files CHANGELOG.md ragframework/retriever/in_memory.py tests/test_retriever/test_in_memory.py— passedgit diff --check— passedType of change