From 96416176da6890939b3a546a7e96d6782ac5a626 Mon Sep 17 00:00:00 2001 From: Jonn Smith Date: Mon, 25 Feb 2019 14:10:50 -0500 Subject: [PATCH 1/2] Fixed Concordance to allow for no variation alleles in the truth data. --- .../tools/walkers/validation/Concordance.java | 7 +- .../ConcordanceIntegrationTest.java | 38 +++++- .../tools/concordance/noVariationAlleles.vcf | 114 ++++++++++++++++++ 3 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 src/test/resources/org/broadinstitute/hellbender/tools/concordance/noVariationAlleles.vcf diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java index 293ee830f63..09f0241f825 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java @@ -5,7 +5,6 @@ import htsjdk.variant.variantcontext.writer.VariantContextWriter; import htsjdk.variant.vcf.*; import org.apache.commons.collections4.Predicate; -import org.apache.commons.lang.mutable.MutableInt; import org.apache.commons.lang.mutable.MutableLong; import org.broadinstitute.barclay.argparser.Argument; import org.broadinstitute.barclay.argparser.BetaFeature; @@ -291,8 +290,12 @@ public Object onTraversalSuccess() { @Override protected boolean areVariantsAtSameLocusConcordant(final VariantContext truth, final VariantContext eval) { final boolean sameRefAllele = truth.getReference().equals(eval.getReference()); + // we assume that the truth has a single alternate allele - final boolean containsAltAllele = eval.getAlternateAlleles().contains(truth.getAlternateAllele(0)); + final boolean containsAltAllele = + (truth.getAlternateAlleles().size() == eval.getAlternateAlleles().size()) && + ((truth.getAlternateAlleles().size() > 0) && + eval.getAlternateAlleles().contains(truth.getAlternateAllele(0))); return sameRefAllele && containsAltAllele; } diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java index 223a98c02b3..7741d75484d 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java @@ -3,12 +3,16 @@ import htsjdk.variant.variantcontext.VariantContext; import org.broadinstitute.hellbender.CommandLineProgramTest; import org.broadinstitute.hellbender.engine.AbstractConcordanceWalker; +import org.broadinstitute.hellbender.exceptions.GATKException; import org.broadinstitute.hellbender.testutils.VariantContextTestUtils; import org.testng.Assert; import org.testng.annotations.Test; import java.io.File; -import java.util.*; +import java.io.IOException; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; import java.util.stream.Collectors; /** @@ -47,7 +51,6 @@ public void testSimple() throws Exception { Assert.assertEquals(indelRecord.getFalseNegatives(), 2); Assert.assertEquals(snpRecord.getSensitivity(), 1.0/4, epsilon); Assert.assertEquals(snpRecord.getPrecision(), 1.0, epsilon); - } // Test going from an integer chromosome (22) to a character chromosome (X) @@ -209,4 +212,35 @@ public void testDreamSensitivity() throws Exception { // 78 Assert.assertEquals(snpRecord.getTruePositives() + snpRecord.getFalseNegatives(), 78); } + + @Test + public void testDoesNotCrashWithNO_VARIATIONAlleles() { + final String testDir = toolsTestDir + "concordance/"; + final File evalVcf = new File(testDir, "noVariationAlleles.vcf"); + final File truthVcf = new File(testDir, "noVariationAlleles.vcf"); + final File summary = createTempFile("summary", ".txt"); + + final String[] args = { + "--" + AbstractConcordanceWalker.EVAL_VARIANTS_LONG_NAME, evalVcf.toString(), + "--" + AbstractConcordanceWalker.TRUTH_VARIANTS_LONG_NAME, truthVcf.toString(), + "--" + Concordance.SUMMARY_LONG_NAME, summary.toString(), + }; + + runCommandLine(args); + + try { + final ConcordanceSummaryRecord.Reader reader = new ConcordanceSummaryRecord.Reader(summary); + final ConcordanceSummaryRecord snpRecord = reader.readRecord(); + final ConcordanceSummaryRecord indelRecord = reader.readRecord(); + + // Some token validation: + Assert.assertEquals(snpRecord.getSensitivity(), 1, 0.005); + Assert.assertEquals(indelRecord.getSensitivity(), 0, 0.005); + Assert.assertEquals(snpRecord.getTruePositives() + snpRecord.getFalseNegatives(), 1); + Assert.assertEquals(indelRecord.getTruePositives() + indelRecord.getFalseNegatives(), 2); + } + catch (final IOException | java.lang.NullPointerException ex) { + throw new GATKException("Could not get summary file! ", ex); + } + } } \ No newline at end of file diff --git a/src/test/resources/org/broadinstitute/hellbender/tools/concordance/noVariationAlleles.vcf b/src/test/resources/org/broadinstitute/hellbender/tools/concordance/noVariationAlleles.vcf new file mode 100644 index 00000000000..1ed1a634237 --- /dev/null +++ b/src/test/resources/org/broadinstitute/hellbender/tools/concordance/noVariationAlleles.vcf @@ -0,0 +1,114 @@ +##fileformat=VCFv4.2 +##FILTER= +##FORMAT= +##FORMAT= +##FORMAT= +##FORMAT= +##FORMAT= +##GATKCommandLine= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##INFO= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##contig= +##source=HaplotypeCaller +#CHROM POS ID REF ALT QUAL FILTER INFO FORMAT NA12878 +7 100550837 . C . 258.77 . AN=2;DP=25;MQ=43.42 GT:AD:DP 0/0:25:25 +7 100550841 . C . 246.77 . AN=2;DP=28;MQ=45.49 GT:AD:DP 0/0:28:28 +7 100550873 . G C 479.77 . AC=1;AF=0.500;AN=2;BaseQRankSum=3.009;DP=38;ExcessHet=3.0103;FS=1.547;MLEAC=1;MLEAF=0.500;MQ=53.19;MQRankSum=0.000;QD=13.71;ReadPosRankSum=0.366;SOR=0.412 GT:AD:DP:GQ:PL 0/1:16,19:35:99:508,0,353 From e49bcdd23a3ec8ad49f8d90fbd32fee588602220 Mon Sep 17 00:00:00 2001 From: Jonn Smith Date: Tue, 26 Feb 2019 11:41:14 -0500 Subject: [PATCH 2/2] Addressing review comments. --- .../tools/walkers/validation/Concordance.java | 3 +- .../ConcordanceIntegrationTest.java | 38 +++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java index 09f0241f825..b33a1ccaef5 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/validation/Concordance.java @@ -291,7 +291,8 @@ public Object onTraversalSuccess() { protected boolean areVariantsAtSameLocusConcordant(final VariantContext truth, final VariantContext eval) { final boolean sameRefAllele = truth.getReference().equals(eval.getReference()); - // we assume that the truth has a single alternate allele + // We make sure that the truth has at least one alt allele. + // If it does, we pick the first for comparison: final boolean containsAltAllele = (truth.getAlternateAlleles().size() == eval.getAlternateAlleles().size()) && ((truth.getAlternateAlleles().size() > 0) && diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java index 7741d75484d..1a99eaa1b51 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/validation/ConcordanceIntegrationTest.java @@ -21,11 +21,12 @@ public class ConcordanceIntegrationTest extends CommandLineProgramTest{ final double epsilon = 1e-3; + private static final String CONCORDANCE_TEST_DIR = toolsTestDir + "concordance/"; + @Test public void testSimple() throws Exception { - final String testDir = toolsTestDir + "concordance/"; - final File evalVcf = new File(testDir, "gatk4-dream3-mini.vcf"); - final File truthVcf = new File(testDir, "gatk3-dream3-mini.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "gatk4-dream3-mini.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "gatk3-dream3-mini.vcf"); final File summary = createTempFile("summary", ".txt"); final String[] args = { @@ -56,9 +57,8 @@ public void testSimple() throws Exception { // Test going from an integer chromosome (22) to a character chromosome (X) @Test public void testt22X() throws Exception { - final String testDir = toolsTestDir + "concordance/"; - final File truthVcf = new File(testDir, "gatk3-dream3-x.vcf"); - final File evalVcf = new File(testDir, "gatk4-dream3-x.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "gatk3-dream3-x.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "gatk4-dream3-x.vcf"); final File tpfp = createTempFile("tpfp", ".vcf"); final File tpfn = createTempFile("tpfn", ".vcf"); final File ftnfn = createTempFile("ftnfn", ".vcf"); @@ -106,9 +106,8 @@ public void testt22X() throws Exception { // The truth file only contains the PASS sites; should get 100% sensitivity and specificity. @Test public void testPerfectMatchVcf() throws Exception { - final String testDir = toolsTestDir + "concordance/"; - final File truthVcf = new File(testDir, "same-truth.vcf"); - final File evalVcf = new File(testDir, "same-eval.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "same-truth.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "same-eval.vcf"); final File summary = createTempFile("summary", ".txt"); final String[] args = { @@ -135,9 +134,8 @@ public void testPerfectMatchVcf() throws Exception { @Test public void testFilterAnalysis() throws Exception { - final String testDir = toolsTestDir + "concordance/"; - final File truthVcf = new File(testDir, "filter-analysis-truth.vcf"); - final File evalVcf = new File(testDir, "filter-analysis-eval.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "filter-analysis-truth.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "filter-analysis-eval.vcf"); final File summary = createTempFile("summary", ".txt"); final File filterAnalysis = createTempFile("filter-analysis", ".txt"); @@ -179,9 +177,8 @@ public void testFilterAnalysis() throws Exception { @Test public void testDreamSensitivity() throws Exception { - final String testDir = toolsTestDir + "concordance/"; - final File evalVcf = new File(testDir, "dream3-chr21.vcf"); - final File truthVcf = new File(testDir, "dream3-truth-minus-SV-chr21.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "dream3-chr21.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "dream3-truth-minus-SV-chr21.vcf"); final File summary = createTempFile("summary", ".txt"); final String[] args = { @@ -215,9 +212,8 @@ public void testDreamSensitivity() throws Exception { @Test public void testDoesNotCrashWithNO_VARIATIONAlleles() { - final String testDir = toolsTestDir + "concordance/"; - final File evalVcf = new File(testDir, "noVariationAlleles.vcf"); - final File truthVcf = new File(testDir, "noVariationAlleles.vcf"); + final File evalVcf = new File(CONCORDANCE_TEST_DIR, "noVariationAlleles.vcf"); + final File truthVcf = new File(CONCORDANCE_TEST_DIR, "noVariationAlleles.vcf"); final File summary = createTempFile("summary", ".txt"); final String[] args = { @@ -229,8 +225,8 @@ public void testDoesNotCrashWithNO_VARIATIONAlleles() { runCommandLine(args); try { - final ConcordanceSummaryRecord.Reader reader = new ConcordanceSummaryRecord.Reader(summary); - final ConcordanceSummaryRecord snpRecord = reader.readRecord(); + final ConcordanceSummaryRecord.Reader reader = new ConcordanceSummaryRecord.Reader(summary); + final ConcordanceSummaryRecord snpRecord = reader.readRecord(); final ConcordanceSummaryRecord indelRecord = reader.readRecord(); // Some token validation: @@ -243,4 +239,4 @@ public void testDoesNotCrashWithNO_VARIATIONAlleles() { throw new GATKException("Could not get summary file! ", ex); } } -} \ No newline at end of file +}