chr1swallace / coloc

Repo for the R package coloc
144 stars 44 forks source link

Clarification of `null_weight` argument passed to `susie_rss()` #77

Closed jdblischak closed 2 years ago

jdblischak commented 2 years ago

Quick summary: I am attempting to run SuSiE myself and then pass the returned SuSiE objects directly to coloc.susie(). Do I need to set null_weight to a non-NULL value when running SuSiE?

Background

The documentation for the argument p of runsusie() states that the default of p=NULL will result in passing a non-NULL value to the argument null_weight of susie_rss():

https://github.com/chr1swallace/coloc/blob/f9635f56bad8a2fef69aceee8f4eb30b62a8e7a0/R/susie.R#L406-L412

Correspondingly, the SuSiE vignette states (source):

runsusie() is a wrapper around susieR::susie_rss(). It sets the null_weight parameter - this must be non-zero or we cannot back calculate Bayes factors needed for coloc

From investigating the GitHub repo, this section of the vignette was prompted by the discussion in https://github.com/chr1swallace/coloc/issues/46#issuecomment-813954242:

I realised we can't set null_weight=NULL, because I need SuSiE to give me a posterior probability for the null hypothesis so I can back-calculate the Bayes factors needed for coloc.

However, the code appears to be out of sync from the documentation above, because null_weight is no longer set by default:

https://github.com/chr1swallace/coloc/blob/ac928af483561ac3d3b3d91b5f631f8679505f89/R/susie.R#L484-L490

And the documentation for the argument back_calculate_lbf of coloc.susie() now states that back-calculating the BFs is for testing only:

https://github.com/chr1swallace/coloc/blob/ac928af483561ac3d3b3d91b5f631f8679505f89/R/susie.R#L99-L102

chr1swallace commented 2 years ago

thanks for spotting this! The vignette is out of date. The null_weight was needed before, but now susie returns bayes factors directly so I don't need this hack.

On Tue, 2022-03-01 at 10:58 -0800, John Blischak wrote:

Quick summary: I am attempting to run SuSiE myself and then pass the returned SuSiE objects directly to coloc.susie(). Do I need to set null_weight to a non-NULL value when running SuSiE? Background The documentation for the argument p of run_susie() states that the default of p=NULL will result in passing a non-NULL value to the argument null_weight of susie_rss(): https://github.com/chr1swallace/coloc/blob/f9635f56bad8a2fef69aceee8f4eb30b62a8e7a0/R/susie.R#L406-L412 Correspondingly, the SuSiE vignette states (source):

runsusie() is a wrapper around susieR::susie_rss(). It sets the null_weight parameter - this must be non-zero or we cannot back calculate Bayes factors needed for coloc From investigating the GitHub repo, this section of the vignette was prompted by the discussion in #46 (comment): I realised we can't set null_weight=NULL, because I need SuSiE to give me a posterior probability for the null hypothesis so I can back-calculate the Bayes factors needed for coloc. However, the code appears to be out of sync from the documentation above, because null_weight is no longer set by default: https://github.com/chr1swallace/coloc/blob/ac928af483561ac3d3b3d91b5f631f8679505f89/R/susie.R#L484-L490 And the documentation for the argument back_calculate_lbf of coloc.susie() now states that back-calculating the BFs is for testing only: https://github.com/chr1swallace/coloc/blob/ac928af483561ac3d3b3d91b5f631f8679505f89/R/susie.R#L99-L102 — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.Message ID: @.***>

jdblischak commented 2 years ago

Thanks for the quick response! Would you like me to send a quick PR to update this?

jdblischak commented 2 years ago

Also relevant to the coloc documentation is that the SuSiE authors are planning on changing the default for null_weight from NULL to 0 https://github.com/stephenslab/susieR/issues/153

jdblischak commented 2 years ago

Looking at the code again for runsusie() (and coloc.susie()), it appears that the argument p for runsusie() is completely ignored. Users have to explicitly set null_weight in order to have any affect on susie_rss().