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

feat: flag and filter credible sets #879

Merged
merged 13 commits into from
Oct 28, 2024

Conversation

Tobi1kenobi
Copy link
Contributor

@Tobi1kenobi Tobi1kenobi commented Oct 25, 2024

✨ Context

This PR adds a study locus QC flag for when the sum of posterior inclusion probabilities are not in the expected range [0.99,1] addresses issues/#3566

🛠 What does this PR implement

  • Add a method to flag abnormal loci for when the sum of PIPs < 0.99 or > 1
  • Adapts tests from another study locus test function to flag both a "bad" and "good" locus

🙈 Missing

🚦 Before submitting

  • Do these changes cover one single feature (one change at a time)?
  • Did you read the contributor guideline?
  • Did you make sure to update the documentation with your changes?
  • Did you make sure there is no commented out code in this PR?
  • Did you follow conventional commits standards in PR title and commit messages?
  • Did you make sure the branch is up-to-date with the dev branch?
  • Did you write any new necessary tests?
  • Did you make sure the changes pass local tests (make test)?
  • Did you make sure the changes pass pre-commit rules (e.g poetry run pre-commit run --all-files)?

@Tobi1kenobi Tobi1kenobi marked this pull request as ready for review October 25, 2024 12:04
Copy link
Contributor

@ireneisdoomed ireneisdoomed left a comment

Choose a reason for hiding this comment

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

Looks good Tobi, thanks! Have a look at my suggestion, I think it is more performant
Thank you for the tests

@@ -45,6 +45,8 @@ def __init__(
.annotate_study_type(study_index) # Add study type to study locus
.qc_redundant_top_hits_from_PICS() # Flagging top hits from studies with PICS summary statistics
.qc_explained_by_SuSiE() # Flagging credible sets in regions explained by SuSiE
# Flagging credible sets with PIP > 1 or PIP < 0.99
.qc_abnormal_pips(sum_pips_lower_threshold=0.99,sum_pips_upper_threshold=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that we are not dropping credible sets due to the lower bound, only the upper?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both below lower or above the upper bound are dropped, in theory - in practice I'm not so sure

@ireneisdoomed
Copy link
Contributor

Do you have an idea of how many CS have this issue?

@Tobi1kenobi
Copy link
Contributor Author

Do you have an idea of how many CS have this issue?

When I last looked at gwas catalog susie I didn't find any credible sets with the issue. I guess @addramir would know more about where this has been a problem in the past

@addramir
Copy link
Contributor

I would be happy if 0 CSs are having this issue. If not - we have a problem.

@Tobi1kenobi
Copy link
Contributor Author

Implemented @ireneisdoomed's suggestion and after cross-checking vs gwas catalog susie I tweaked the threshold to be 1.0001 rather than 1 as there were some sums creeping through likely due to floating point error.

Copy link
Contributor

@ireneisdoomed ireneisdoomed left a comment

Choose a reason for hiding this comment

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

Thank you!

# Flagging loci with failed studies:
.withColumn(
"qualityControls",
self.update_quality_flag(
qc_select_expression,
f.col("sumPosteriorProbability").isNotNull(),
f.col("pipOutOfRange") == "outside",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this could be a boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True! Done!

@Tobi1kenobi Tobi1kenobi merged commit d12d65d into dev Oct 28, 2024
5 checks passed
@Tobi1kenobi Tobi1kenobi deleted the alegbe-flag_and_filter_credible_sets branch October 28, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flagging and filtering CSs with sum of PIPs not in [0.99,1]
3 participants