-
Notifications
You must be signed in to change notification settings - Fork 103
Experimental Feature: Function delegation to rapids_singlecell when gpu is availible #1093
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1093 +/- ##
==========================================
+ Coverage 66.17% 66.21% +0.04%
==========================================
Files 44 45 +1
Lines 7160 7202 +42
Branches 1218 1225 +7
==========================================
+ Hits 4738 4769 +31
- Misses 1944 1951 +7
- Partials 478 482 +4
🚀 New features to boost your workflow:
|
…en/squidpy into feat/device-settings
for more information, see https://pre-commit.ci
…en/squidpy into feat/device-settings
| if effective_device == "gpu": | ||
| from rapids_singlecell.squidpy import co_occurrence as rsc_co_occurrence | ||
|
|
||
| return rsc_co_occurrence( | ||
| adata, | ||
| cluster_key=cluster_key, | ||
| spatial_key=spatial_key, | ||
| interval=interval, | ||
| copy=copy, | ||
| n_splits=n_splits, | ||
| n_jobs=n_jobs, | ||
| backend=backend, | ||
| show_progress_bar=show_progress_bar, | ||
| ) |
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.
I think this can generally be wrapped up in a decorator where we:
- check
effective_device - Subset the inbound arguments from squidpy to those of rapids' function (I imagine
n_jobsdoesn't apply, this can probably be done with python'sinspectmodule) - run the rapids function
- Optionally bring the data back to the CPU (I would guess people want this, or at least it should be behind a flag)
|
Anndata’s settings are indeed much nicer than scanpy’s. They do need a few parts to work, but with all that in place they’re great!
|
|
Ok knowing what you guys would want was important thanks. Currently I will go with the inspect module and a decorator even though I think we might not be able to support 1-1 dispatch in certain configurations in the future. Also will probably have some friction on docstrings and type-hints if we are going to find common arguments dynamically.But lets see.
yeah these are also important to me so I will ensure this. |
|
Before thinking more about settings. I'd like to clarify that we might even want to do this on a fork of rsc, this is the decision we made with @timtreis in order to speed up things. But I really want to make sure we don't diverge too much and it will be in a state where we can incorperate our changes to rsc back easily. I think this direction would be good for the health of both repos. I have a plan for squidpy functions like
Another benefit: Power users are important and will keep this repo alive. In general if it wasn't for |
hi @flying-sheep @ilan-gold @timtreis @Intron7 , for squidpy 2.0 we are planning to delegate to the gpu functionality when there is a device present by default. I am trying to come up with an API for this. I saw that the scanpy settings class was very complicated so I though maybe we need something simpler for our case.
This is what I came up with. I will come up with the
co_occurencesupport after lunch but I wanted an initial feedback on this.Example usage
Installation instructions