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

QC for AraTha DFE #1647

Merged
merged 1 commit into from
Jan 10, 2025
Merged

Conversation

clararehmann
Copy link
Contributor

@clararehmann clararehmann commented Jan 8, 2025

Closes #1344.

I used the gamma distribution parameters from the additive A. lyrata model from Table S4 of Huber et al., 2018 to QC the model.

The scale parameter of the distribution is misidentified as gamma_mean in the catalog, so I modified the Gamma_H18 DFE in the AraTha catalog to match so that gamma_mean is now the product of the shape and scale parameters.

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.84%. Comparing base (a344665) to head (33cbce7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1647   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files         133      133           
  Lines        4569     4580   +11     
  Branches      472      472           
=======================================
+ Hits         4562     4573   +11     
  Misses          3        3           
  Partials        4        4           

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

@petrelharp
Copy link
Contributor

I've discussed this with @clararehmann; here's the summary.

As discussed in #1344, this DFE is arguably not the "best" one to use from this paper. However, it is a totally reasonable one, and who says we need just one DFE. So, the plan is to (1) add this additive DFE (and, this PR finalizes that); and (2) also add the "genome-wide both species h-s relationship" DFE (I will open another issue for that).

With that in mind: @clararehmann - how about let's change the name of this DFE, maybe to GammaAdditive_H18, as we discussed? (Or, make another proposal?)

@petrelharp
Copy link
Contributor

petrelharp commented Jan 8, 2025

Ah, uh-oh, @clararehmann - maybe there is a factor of 2 missing as well? Note that in that PR I agreed with you about the parameter being the scale, not the shape.

@petrelharp petrelharp mentioned this pull request Jan 8, 2025
id = "Gamma_H18"
description = "Deleterious Gamma DFE"
id = "GammaAdditive_H18"
description = "Deleterious additive Gamma DFE"
long_description = """
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah - could you update the long description, also? How about

    Additive, deleterious DFE from Huber et al. (2018) for Arabidopsis, 
    estimated using <software> from A. lyrata as a Gamma distribution
    of deleterious effects. Parameters are from Supplementary Table 4,
    the "genome-wide, additive-only model for A. lyrata". The DFE for
    lyrata (rather than thaliana) is provided due to challenges with
    simulating selfing.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I see <software> wasn't some named software package, they just did it using Methods from the paper. I rephrased this to "using the SFS"

stdpopsim/qc/AraTha.py Outdated Show resolved Hide resolved
…del)

gamma_scale was misidentified as gamma_mean
@petrelharp
Copy link
Contributor

Looks good! I've moved the improved description to the catalog, rather than the QC implementation (which no one will ever see); and squashed the commits. When the tests finish we can merge (assuming I didn't muck something up.)

@petrelharp petrelharp merged commit 3f3643c into popsim-consortium:main Jan 10, 2025
11 checks passed
@clararehmann clararehmann deleted the arathal_QC branch January 22, 2025 22:19
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.

QC for AraThal DFE
2 participants