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

De delta eps #1043

Merged
merged 15 commits into from
May 6, 2021
Merged

De delta eps #1043

merged 15 commits into from
May 6, 2021

Conversation

PierreBoyeau
Copy link
Contributor

Fixes #1042

@codecov
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Attention: Patch coverage is 93.61702% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.71%. Comparing base (0f860bf) to head (51dcc8b).

Files Patch % Lines
scvi/utils/_differential.py 93.61% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
+ Coverage   90.70%   90.71%   +0.01%     
==========================================
  Files          90       90              
  Lines        6615     6658      +43     
==========================================
+ Hits         6000     6040      +40     
- Misses        615      618       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

scvi/utils/_differential.py Outdated Show resolved Hide resolved
scvi/utils/_differential.py Show resolved Hide resolved

if where_zero_a.sum() >= 1:
artefact_scales_a = max_scales_a[where_zero_a]
eps_a = np.percentile(artefact_scales_a, q=90)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe should make this percentile an argument with default value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this should be a parameter in get_bayes_factors? I replaced this default value in estimate_pseudocounts_offset but not in get_bayes_factors yet

@PierreBoyeau
Copy link
Contributor Author

I renamed eps to pseudocounts, because I think that the eps function for BF computation is different from the one would use as pseudocounts.
In the latter case, the objective is to zero out gene expression differences that come from softmax artifacts.

scvi/utils/_differential.py Outdated Show resolved Hide resolved
scvi/utils/_differential.py Outdated Show resolved Hide resolved
scvi/utils/_differential.py Outdated Show resolved Hide resolved
tests/core/test_differential.py Outdated Show resolved Hide resolved
tests/core/test_differential.py Outdated Show resolved Hide resolved
scvi/utils/_differential.py Outdated Show resolved Hide resolved
scvi/utils/_differential.py Outdated Show resolved Hide resolved
@adamgayoso adamgayoso merged commit 99fbac4 into master May 6, 2021
@adamgayoso adamgayoso deleted the DE_delta_eps branch May 6, 2021 21:14
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.

Automatic effect size threshold and pseudo-counts offset for DE
2 participants