broadinstitute / gatk

Official code repository for GATK versions 4 and up
https://software.broadinstitute.org/gatk
Other
1.68k stars 589 forks source link

ReferenceConfidenceVariantContextMerger.merge() should use remapped alleles when genotyping #8317

Open droazen opened 1 year ago

droazen commented 1 year ago

Currently ReferenceConfidenceVariantContextMerger.merge() does not pass in the remapped alleles to GATKVariantContextUtils.makeGenotypeCall() for the originalGT argument, which seems problematic and could cause the code to incorrectly fall back on reference calls in some cases. The GVS team has found it necessary to use the remapped alleles when genotyping in order to get correct genotypes out of the merge -- we should make this behavior the default.

Currently, making it the default causes failures in the tests for the --include-non-variant-sites argument in GenotypeGVCFs (see https://github.com/broadinstitute/gatk/pull/8288#issuecomment-1509295869). This appears related to the current implementation incorrectly calculating the reference allele when we are at some non-zero offset from the start of a GVCF reference block.

droazen commented 1 year ago

See https://github.com/broadinstitute/gatk/pull/8318 for an implementation of the desired behavior.