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

destVI bugs #1320

Closed
romain-lopez opened this issue Jan 31, 2022 · 8 comments
Closed

destVI bugs #1320

romain-lopez opened this issue Jan 31, 2022 · 8 comments
Assignees
Labels

Comments

@romain-lopez
Copy link
Member

We have identified several issues with the current version of the DestVI code:

  1. (critical) the current code doesn't train at all on master. Can has flagged an issue with the nobs that needs to be resolved
  2. (important, minor) there is an undesirable setup on the neural net architecture of the amortization network for the proportions (not for the gamma used throughout the paper)
  3. (important, minor) there is some discrepancy in the treatment of the library size that must be figured out in between CondSCVI and DestVI
@canergen
Copy link
Member

canergen commented Jan 31, 2022

1 is fixed by branch fix_elbo from Adam this morning https://github.com/YosefLab/scvi-tools/tree/fix_elbo (still awaiting code review)
2 I will try with simulated data and open a branch afterwards
3 Currently running some tests on using not-log library size or log library size for both models

@canergen
Copy link
Member

canergen commented Jan 31, 2022

3 Using the lymph node dataset from the preprint and theory, we decided to change the library size in condVI to not using logarithm.

@adamgayoso adamgayoso added this to the 0.15.0 milestone Feb 1, 2022
@adamgayoso adamgayoso removed this from the 0.15.0 milestone Feb 11, 2022
@romain-lopez
Copy link
Member Author

Now that some of these items are (almost) decided, let's plan on starting a PR after our last chat. Additionally, let's plan to add the sparsity regularizer of cell type proportion into this PR. Making progress!

@canergen
Copy link
Member

It's now addressed in #1457.

@giovp
Copy link
Member

giovp commented Apr 5, 2022

not sure if related but trying destVI on a new dataset following the tutorial, I get kernel error at this stage:

sc_model.train(
    max_epochs=250,
)

Other times instead, can't reproduce, I get nan in the encoder output (again trying to reproduce but can't rn). Few months ago it was working fine though. Will try if #1457 fix it.

@giovp
Copy link
Member

giovp commented Apr 5, 2022

ok, having the same issue with Stereoscope implementation, I'm afraid is independent. Am currently on a mac. Apologies for hijacking the issue.

@canergen
Copy link
Member

canergen commented Apr 5, 2022

The reason for the PR is not numerical instability but instability for estimated cell type proportions with two bug . If it is also an issue with Stereoscope, I would suggest to check the input data first (single dataset where this is happening) and you might want to reduce learning rate in CondSCVI

@adamgayoso
Copy link
Member

This should have been closed by #1457

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

No branches or pull requests

4 participants