Skip to content
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

Coerce to Csparsematrix for C++ conversion efficiency #164

Merged
merged 4 commits into from
Sep 25, 2020

Conversation

scottgigante
Copy link
Contributor

No description provided.

@scottgigante
Copy link
Contributor Author

@LuckyMD bumping this

@scottgigante
Copy link
Contributor Author

Addresses MarioniLab/scran#70

@LuckyMD
Copy link
Collaborator

LuckyMD commented Sep 22, 2020

Sorry, haven't gotten around to this. The point about the 2e9 non-zero value limitation... is that sth we need to worry about?

@LuckyMD
Copy link
Collaborator

LuckyMD commented Sep 22, 2020

2e9 non-zero values would mean 25k genes and 80k cells max. That doesn't really work for single-cell data, no? If you assum 90% dropout that would be 800k cells. Still not really enough for our 1 million cell case. What do you think about a size limitation for when you cast to dgCmatrix?

@scottgigante
Copy link
Contributor Author

You're right. Maybe a better solution would be to do this in python.

X = adata.X.T
if scipy.sparse.issparse(X):
    if X.nnz > 2**31-1:
        X = X.tocoo()
    else:
        X = X.tocsc()
ro.global_ev['data_mat'] = X

That way we avoid dgRMatrix which is terrible, and use dgCMatrix when possible.

@LuckyMD
Copy link
Collaborator

LuckyMD commented Sep 25, 2020

Could you merge master into this PR? Then I can merge. Looks good now and gives me the same results. We can always troubleshoot if there are storage limitations at some point.

@LuckyMD LuckyMD merged commit 20b18f1 into theislab:master Sep 25, 2020
@LuckyMD
Copy link
Collaborator

LuckyMD commented Sep 25, 2020

no merge conflicts... so all good.

@scottgigante
Copy link
Contributor Author

scottgigante commented Sep 25, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants