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

Fixing bug in FilterMutectCalls for GVCFs #8458

Merged
merged 2 commits into from
Aug 9, 2023
Merged

Conversation

meganshand
Copy link
Contributor

When using GVCFs with Mutect2 (for example with the Mitochondria mode), in the filtering step ADs for symbolic alleles are set to 0 so it doesn't contribute to overall AD. There was an off-by-one error that removed the alt allele AD rather than the <NON_REF> allele AD. This led to NaNs and errors when a site had no ref reads (for example a GT of [ref,alt,<NON_REF>] and AD of [0,300,0] would accidentally be changed to an AD of [0,0,0] if the alt index was removed instead of the <NON_REF> index).

The test changes the AD in one of the sites of the NA12878.MT.g.vcf to have 0 ref reads which fails without the fix in SomaticClusteringModel.

This addresses #8455.

Copy link
Contributor

@ahaessly ahaessly left a comment

Choose a reason for hiding this comment

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

👍 just one comment

// set tumorAD to 0 for symbolic alleles so it won't contribute to overall AD
symIndexes.forEach(i -> tumorADs[i] = 0);
// set tumorAD to 0 for symbolic alleles so it won't contribute to overall AD, ADs include reference allele so add one to the index
symIndexes.forEach(i -> tumorADs[i+1] = 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I just want to make sure that this can't try to read past the end of the array. I think that the non-ref allele is always at the end and as long as that is not a symbolic allele, I think you should be ok. Just want to make sure that's correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be correct, but would it be better to put an Assert in for the lengths of the Arrays?

@meganshand meganshand merged commit 618f3b1 into master Aug 9, 2023
@meganshand meganshand deleted the ms_mt_filtering_bug branch August 9, 2023 18:28
rickymagner pushed a commit that referenced this pull request Nov 28, 2023
* fixing bug in FilterMutectCalls for GVCFs

* adding assert
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.

2 participants