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

single sample fast mode for qual is HC isActive #6343

Merged
merged 2 commits into from Jan 7, 2020

Conversation

davidbenjamin
Copy link
Contributor

Closes #6111.

The qual used in HC's isActive method now matches that of GenotypeGVCFs. That is, they both now use the new qual. This lets us finally delete AFPriorProvider. I have profiled and HaplotypeCaller speeds up in this branch by a negligible but measurable amount.

@ldgauthier Can you look at this? the big paragraph changed in the new qual notes is just correcting the omission of some -1s and has nothing to do with the single-sample calculation.

Copy link
Contributor

@ldgauthier ldgauthier left a comment

Choose a reason for hiding this comment

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

One minor quibble over terminology in javadoc, otherwise LGTM. Congrats on removing the last of the old qual. :-D

* for the single-sample diploid case.
*/
@Advanced
@Argument(fullName = "input-prior", doc = "Input prior for calls", optional = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh thank goodness this is gone.

@@ -154,6 +155,30 @@ public AFCalculationResult calculate(final VariantContext vc, final int defaultP
return new AFCalculationResult(integerAltAlleleCounts, alleles, log10PNoVariant, log10PRefByAllele);
}

/**
* Calculate the posterior probability that a single biallelic sample is non-ref
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the phrase biallelic sample, but I get that the gist is single-sample. Maybe biallelic genotype?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -292,6 +293,36 @@ public void testSpanningDeletionWithVeryUnlikelyAltAllele() {
final double log10PVariant = afCalc.calculate(vc).log10ProbVariantPresent();
}

//check the exact integration calculation
@Test
Copy link
Contributor

Choose a reason for hiding this comment

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

Lovely tests, as always.

@ldgauthier ldgauthier self-requested a review January 7, 2020 15:12
Copy link
Contributor

@ldgauthier ldgauthier left a comment

Choose a reason for hiding this comment

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

:shipit:

@davidbenjamin davidbenjamin merged commit 6765e0e into master Jan 7, 2020
@davidbenjamin davidbenjamin deleted the db_qual_optimization branch January 7, 2020 16:24
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.

Last relic of old qual
2 participants