-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(performance): speedup for zarr
-based sparse indexing
#1790
Conversation
Benchmark changes
Comparison: https://github.com/scverse/anndata/compare/d055963dc70915ad921965a03d2b7342a098dd6b..e45a05f785aa361068c06b92a3070190fd5ed25f More details: https://github.com/scverse/anndata/pull/1790/checks?check_run_id=34197556781 |
Very good :) |
zarr
-based sparse indexingzarr
-based sparse indexing
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1790 +/- ##
==========================================
- Coverage 87.04% 84.57% -2.47%
==========================================
Files 40 40
Lines 6089 6095 +6
==========================================
- Hits 5300 5155 -145
- Misses 789 940 +151
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great! I think for clarity, wherever possible you should unpack an entry of indptr_list
into start, end
instead of using s[0]
and s[1]
.
you could either do indptr_limits = [(x.indptr[s.start], x.indptr[s.stop + 1]) for s in slices]
or so, or just define indptr_slices
as above.
…-based sparse indexing) (#1801) Co-authored-by: Ilan Gold <[email protected]>
On my computer, this is a 3-5X speedup.