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

Poissonmultivi #2735

Closed
wants to merge 15 commits into from
Closed

Conversation

lauradmartens
Copy link
Contributor

Hi @martinkim0,

I had some time and added the Poisson loss also to the MultiVI model (POISSONMULTIVI) to the external models.

Addresses #2257 for Poisson likelihood.

@lauradmartens
Copy link
Contributor Author

I added the option to have separate size factors for RNA and ATAC, not sure if this is the best way.

Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 78.36991% with 138 lines in your changes missing coverage. Please review.

Project coverage is 84.34%. Comparing base (cfa3d83) to head (1105a0c).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/scvi/external/poissonmultivi/_model.py 72.28% 74 Missing ⚠️
src/scvi/external/poissonmultivi/_module.py 82.51% 64 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2735      +/-   ##
==========================================
- Coverage   84.60%   84.34%   -0.26%     
==========================================
  Files         178      181       +3     
  Lines       15052    15690     +638     
==========================================
+ Hits        12734    13234     +500     
- Misses       2318     2456     +138     
Files with missing lines Coverage Δ
src/scvi/_constants.py 100.00% <100.00%> (ø)
src/scvi/external/__init__.py 100.00% <100.00%> (ø)
src/scvi/external/poissonmultivi/__init__.py 100.00% <100.00%> (ø)
src/scvi/external/poissonmultivi/_module.py 82.51% <82.51%> (ø)
src/scvi/external/poissonmultivi/_model.py 72.28% <72.28%> (ø)

@martinkim0 martinkim0 self-assigned this Apr 22, 2024
@martinkim0 martinkim0 added the P2 label Jul 12, 2024
@canergen
Copy link
Member

Quick check looks good. Can you explain, why we don't inherit from MULTIVI and MULTIVAE?

@ori-kron-wis ori-kron-wis added this to the scvi-tools 1.2 milestone Nov 20, 2024
@ori-kron-wis ori-kron-wis added the on-merge: backport to 1.2.x on-merge: backport to 1.2.x label Nov 20, 2024
@canergen
Copy link
Member

Closing here due to inactivity. We have our own proposed Poisson distribution in multiVI that will be merged in the near future.

@canergen canergen closed this Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on-merge: backport to 1.2.x on-merge: backport to 1.2.x P2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants