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.

Sign up for GitHub

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

Jump to bottom

To scvi tools #61

Merged
merged 15 commits into from
Jun 22, 2021
Merged

To scvi tools #61

merged 15 commits into from
Jun 22, 2021

Conversation

njbernstein
Copy link
Contributor

@njbernstein njbernstein commented Feb 18, 2021 •
edited
Loading

Here's the update for moving to scvi-tools. Things mostly moved over smoothly.

Notable changes:

  1. Removed other doublet creation methods (doublet creation method lives in scvi-tools now)
  2. Removed experimental doublets option. I think this is something the end-user can handle.
  3. The overall training scheme is changed for the vae. We use a tunable learning rate which changes after validation reaches a plateau
  4. Moved to the black code style format.
  5. Removed ability to go directly from 10x data as scvi-tools no longer directly supports it

Got increased performance on pbmcs and lower performance on kidney.
Output for performance script:

Mean AUROC has changed by for kidney: -0.0027609993290108603
P value for metric change AUROC in experiment kidney: 0.08545176011539873
Mean average_precision has changed by for kidney: -0.007709500685913362
P value for metric change average_precision in experiment kidney: 0.027617126859031914
WARNING!
WARNING!
WARNING!
WARNING!
WARNING!
WARNING average_precision HAS GOTTEN SIGNIFICANTLY WORSE for kidney!
Mean AUROC has changed by for pbmc: -0.008306661482196498
P value for metric change AUROC in experiment pbmc: 0.0025374340489701264
WARNING!
WARNING!
WARNING!
WARNING!
WARNING!
WARNING AUROC HAS GOTTEN SIGNIFICANTLY WORSE for pbmc!
Mean average_precision has changed by for pbmc: 0.014607552525554901
P value for metric change average_precision in experiment pbmc: 0.010120285288538741
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB average_precision HAS GOTTEN SIGNIFICANTLY BETTER for pbmc!

Updating of softmax scores doesnt really work anymore. Softmax scores get squished to below .1. Still looking into this

@njbernstein njbernstein linked an issue Feb 18, 2021 that may be closed by this pull request
moving to scvi-tools #60
Closed
@njbernstein
Copy link
Contributor Author

This code depends on scverse/scvi-tools#933 being accepted

adamgayoso
adamgayoso reviewed Feb 19, 2021
View reviewed changes
solo/solo.py Outdated Show resolved Hide resolved
solo/solo.py Outdated Show resolved Hide resolved
solo/solo.py Outdated Show resolved Hide resolved
@njbernstein njbernstein requested a review from davek44 February 19, 2021 01:50
@njbernstein njbernstein assigned njbernstein and unassigned davek44 Feb 19, 2021
davek44
davek44 reviewed Feb 22, 2021
View reviewed changes
solo/solo.py Outdated Show resolved Hide resolved
davek44
davek44 reviewed Feb 22, 2021
View reviewed changes
solo/solo.py Outdated Show resolved Hide resolved
davek44
davek44 reviewed Feb 22, 2021
View reviewed changes
solo/solo.py Show resolved Hide resolved
@njbernstein
Copy link
Contributor Author

Ball park same results as before:

Mean AUROC has changed by for kidney: 0.004320676590979344
P value for metric change AUROC in experiment kidney: 0.027617126859031914
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB AUROC HAS GOTTEN SIGNIFICANTLY BETTER for kidney!
Mean average_precision has changed by for kidney: 0.00572280091958699
P value for metric change average_precision in experiment kidney: 0.00685541503906661
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB!
NICE JOB average_precision HAS GOTTEN SIGNIFICANTLY BETTER for kidney!
Mean AUROC has changed by for pbmc: -0.0002767180325944363
P value for metric change AUROC in experiment pbmc: 0.32403843406957306
Mean average_precision has changed by for pbmc: -0.029941475393492678
P value for metric change average_precision in experiment pbmc: 0.00405655863278289
WARNING!
WARNING!
WARNING!
WARNING!
WARNING!
WARNING average_precision HAS GOTTEN SIGNIFICANTLY WORSE for pbmc!

@njbernstein
Copy link
Contributor Author

@davek44 can you take a look at this?

Recalibration is now not done by default, but I've left the option in

@davek44
Copy link
Collaborator

davek44 commented Jun 15, 2021

Looks good to me. One thought-- does running solo.train twice choose a different train/validation split the second time?

@njbernstein
Copy link
Contributor Author

@davek44 ah good point. I believe it does. Let me see what the best fix for that is

@davek44
Copy link
Collaborator

davek44 commented Jun 15, 2021

They added learning rate plateau helper code at some point, which we should be able to use. See https://github.com/YosefLab/scvi-tools/blob/master/scvi/train/_trainingplans.py#L186

@adamgayoso
Copy link

Looks good to me. One thought-- does running solo.train twice choose a different train/validation split the second time?

It uses the scvi.settings.seed so should be the same each time.

They added learning rate plateau helper code at some point, which we should be able to use.

It would be possible to add, though slightly more complicated

@njbernstein
Copy link
Contributor Author

sweet, so no need to change that part. Any other issues @davek44 ?

@davek44
Copy link
Collaborator

davek44 commented Jun 22, 2021

Yea that's all that I see that looked questionable. Go ahead

@njbernstein njbernstein merged commit a3de50e into master Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamgayoso adamgayoso adamgayoso left review comments

@davek44 davek44 davek44 left review comments

Assignees

@njbernstein njbernstein

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

moving to scvi-tools
3 participants
@njbernstein @davek44 @adamgayoso