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

PyTorch and pandas syntax fixes and expression matrix performance improvement #24

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

y1zhou
Copy link

@y1zhou y1zhou commented Feb 7, 2022

Hi @changwn, I've been trying to use scFEA on some of my datasets, and updated the main module src/scFEA.py to get it working on my end. Here's a list of the modifications I made:

  • Formatted all files under src/ with black. This is a side-effect of how my editor is set up and I was too many edits in to revert it.
  • pandas and torch were raising deprecation warnings on some of the functions/methods used. It might be a good idea to lock the versions in your requirements.txt. I have pandas 1.4.0 and pytorch 1.10.2 in my environment and they don't seem to match your versions.
  • The FLUX class had a matrix parameter in its constructor method but was never used. I'm assuming your Dataset class handles the input data already?
  • In your main function making geneExprDf was a slow step. You were copying geneExpr into temp in each iteration and transposing temp, so by simply taking the transpose outside of the loop improves performance by over 50% on your test dataset.
    You may also be able to avoid the entire loop by constructing geneExprDf through a table join of moduleGene and geneExpr, followed by pivoting the joined table from long to wide. Not sure about the performance of this method when geneExpr is huge though.

@changwn
Copy link
Owner

changwn commented Feb 9, 2022

Hi @y1zhou,

This looks amazing. I will merge it into my branch after testing it on my side. Thanks.

Wennan

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