Chunk OrthogonalProcrustesAlignment top_k search to cut peak memory - #309
Open
AmitSubhash wants to merge 1 commit into
Open
Chunk OrthogonalProcrustesAlignment top_k search to cut peak memory#309AmitSubhash wants to merge 1 commit into
OrthogonalProcrustesAlignment top_k search to cut peak memory#309AmitSubhash wants to merge 1 commit into
Conversation
…ControlLab#307) fit() materialized the full (n_label, n_ref) distance matrix and its row-wise argsort before selecting top_k references, so peak memory was O(n_label * n_ref) regardless of subsample. Search top_k in blocks of label rows: each block is scored against all references, so the per-row selection is bit-identical to the unchunked path (ties included), while peak memory drops to O(block * n_ref). On a 15000x15000 case this is 5.4 GB -> 0.6 GB. The block size is internal, derived from a module-level constant, so there is no public API change. Addresses onford's report; regression tests compare the chunked path against an independent full-matrix reference across shapes, top_k (including top_k == n_ref), 1D/2D labels and ties, and assert the search really runs in bounded blocks.
|
Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with |
Author
|
@cla-bot check |
|
Thanks for tagging me. I looked for a signed form under your signature again, and updated the status on this PR. If the check was successful, no further action is needed. If the check was unsuccessful, please see the instructions in my first comment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #307.
Problem
OrthogonalProcrustesAlignment.fitmaterializes the full(n_label, n_ref)distance matrix and its row-wiseargsortbefore selecting thetop_kclosest references.subsampleis only applied afterwards, so it cannot reduce peak memory: peak staysO(n_label * n_ref)no matter how smallsubsampleis. That is the blow-up @onford reported.Fix
Run the
top_ksearch in blocks oflabelrows. Each block is scored against all references, so the per-row selection is unchanged while peak memory drops toO(block * n_ref).Because the chunking is over rows only, never over the feature/contraction dimension, each row's distance vector is computed exactly as before. The result is therefore bit-identical to the previous implementation, ties included.
The block size is internal (derived from a module-level constant), so there is no public API change. I'd floated a
chunk_sizeconstructor parameter in #307, happy to add it if you'd prefer, but the fix doesn't need user tuning so I kept the surface minimal.Measured effect
15000 x 15000alignment (top_k=5,subsample=1000), peak RSS:roughly 8.6x lower.
Tests
Added to
tests/test_data_helper.py:test_orthogonal_alignment_chunking_matches_full— parametrized over shapes,top_k(includingtop_k == n_ref) and 1D/2D labels. Compares against an independently computed full-matrix reference, then asserts the multi-block and single-block runs are bit-identical (assert_array_equal) for both_transformandtransform(data).test_orthogonal_alignment_chunking_bit_identical_with_ties— duplicated integer labels give tied distances; asserts bit identity.test_orthogonal_alignment_chunking_bounds_block_size— asserts the search really runs in bounded blocks (spies on_distance). I checked that reverting to the old full-matrix path makes only this test fail, so it guards the memory contract rather than just re-testing equivalence.pytest tests/test_data_helper.py -m "not requires_dataset"(46 passed) and thehelper.pydoctests pass;yapfandisortare clean.