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

Update FAF help page #1276

Merged
merged 5 commits into from
Nov 13, 2023
Merged

Update FAF help page #1276

merged 5 commits into from
Nov 13, 2023

Conversation

ch-kr
Copy link
Contributor

@ch-kr ch-kr commented Nov 8, 2023

…bout group with highest FAF, not FAF calculated on group with highest AF
@ch-kr ch-kr requested a review from klaricch November 8, 2023 19:28
browser/help/topics/faf.md Outdated Show resolved Hide resolved
browser/help/topics/faf.md Outdated Show resolved Hide resolved

This annotation (abbreviated "`grpmax`") contains allele frequency information (AC, AN, AF, homozygote count) for the non-bottlenecked genetic ancestry groups with the highest frequency.
This annotation (abbreviated "`grpmax FAF`") contains filtering allele frequency information (AC, AN, AF, homozygote count) for the non-bottlenecked genetic ancestry groups with the highest FAF. Note that this field contains filtering allele frequency information from the genetic ancestry group with the highest **FAF**, not the filtering allele frequency information calculated on the genetic ancestry group with the highest **AF**.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we actually provide AC, AN, AF, homozygote count for the grpmax FAF? In the tables it looks like just faf95/99_max and faf95/99_max_gen_anc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, good catch! this text is old and probably describes the grpmax struct

Choose a reason for hiding this comment

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

@klaricch @ch-kr , I was looking at the v4 genomes tables earlier and trying to figure out the precise difference between the grpmax and fafmax values. It looks like fafmax.faf95_max matches what it on the browser UI labeled Grpmax Filtering AF More information(95% confidence)

And grpmax.AF matches the Allele Frequency (non-filtering) row on the browser UI corresponding to grpmax.gen_anc.

But this note on the browser UI calls the "Grpmax Filtering AF" just grpmax (called fafmax in the hail table), so I think there is some inconsistency between the term grpmax on the browser (in this note) and in the hail table and in the hail table documentation here: https://gnomad.broadinstitute.org/help/v4-hts#annotation-descriptions

Copy link
Contributor Author

@ch-kr ch-kr Nov 13, 2023

Choose a reason for hiding this comment

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

hmm, it looks like one of the changes in this PR didn't get applied:

The filtering allele frequency calculation only includes non-bottlenecked genetic ancestry groups: we did not calculate this metric on the Amish (`ami`), Ashkenazi Jewish (`asj`), European Finnish (`fin`), Middle Eastern (`mid`), and "Remaining Individuals" (`rmi`) groups.

On the browser, this annotation is directly available on the variant page. In the VCF and Hail Tables, this annotation (abbreviated "`faf`") is computed globally and for each genetic ancestry group. Filtering allele frequencies (FAFs) for each genetic ancestry group specific are listed separately with 95% and 99% CIs.

image

The text in the browser UI (this section):
image
should show that the text describing the filtering allele frequency applies to the fafmax annotation in the VCF and Hail Tables. The section below
image
now describes grpmax, which is distinct from fafmax.

I'd welcome any updates if you have suggestions for how to make this text clearer!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

@ch-kr Ah I see maybe I was looking at an out of date version of the code. It looks like maybe the browser has been partially updated with these changes but not completely.

One thing that could be improved is to make it totally clear that the field Grpmax Filtering AF that this tooltip is displayed on in the browser, is distinct from Group maximum allele frequency (also described in this tooltip) in that one is filtering and one is not.

It seems like there are 4 concepts:

  1. AF: allele frequency (raw, overall + per-group)
  2. grpmax AF: grpmax allele frequency (raw, only top group)
  3. FAF: filtering allele frequency (filtered, overall + per-group)
  4. grpmax FAF: grpmax filtering allele frequency (filtered, only top group)

The gnomad browser at the top shows 1 (AF) and 4 (grpmax FAF), and 2 (grpmax AF) can be looked up in the Genetic Ancestry Group Frequencies table on the page. And 3 is not available on the browser.

And the hail tables call 1 -> AF/freq, 2 -> grpmax, 3 -> faf, and 4 -> fafmax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can open an additional PR adding a line distinguishing grpmax from FAF (would you mind reviewing @theferrit32 ?)

for your second point -- these fields (freq, grpmax, faf) are described on this help page: https://gnomad.broadinstitute.org/help/v4-hts. is there text you'd like added there as well?

@klaricch klaricch self-assigned this Nov 9, 2023
@ch-kr ch-kr requested a review from klaricch November 13, 2023 14:55
Copy link
Contributor

@klaricch klaricch left a comment

Choose a reason for hiding this comment

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

lgtm!

@rileyhgrant rileyhgrant merged commit 0eb6920 into main Nov 13, 2023
1 check passed
@rileyhgrant rileyhgrant deleted the faf branch November 13, 2023 19:53
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.

4 participants