More informative error messages when dictionaries don't match (#1870)

kachulis · web-flow · commit e67fe94af52b · 2023-05-11T12:19:40.000-04:00
diff --git a/src/main/java/picard/analysis/CollectOxoGMetrics.java b/src/main/java/picard/analysis/CollectOxoGMetrics.java
@@ -40,12 +40,12 @@
 import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
 import org.broadinstitute.barclay.help.DocumentedFeature;
 import picard.PicardException;
-import htsjdk.samtools.util.SequenceUtil;
 import picard.cmdline.CommandLineProgram;
 import picard.cmdline.StandardOptionDefinitions;
 import picard.cmdline.programgroups.DiagnosticsAndQCProgramGroup;
 import picard.util.DbSnpBitSetUtil;
 import picard.util.help.HelpConstants;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.File;
 import java.util.*;
@@ -239,8 +239,11 @@ protected int doWork() {
         final SamReader in = SamReaderFactory.makeDefault().referenceSequence(REFERENCE_SEQUENCE).open(INPUT);
 
         if (!in.getFileHeader().getSequenceDictionary().isEmpty()) {
-            SequenceUtil.assertSequenceDictionariesEqual(in.getFileHeader().getSequenceDictionary(),
-                    refWalker.getSequenceDictionary());
+            SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                    in.getFileHeader().getSequenceDictionary(),
+                    INPUT.getAbsolutePath(),
+                    refWalker.getSequenceDictionary(),
+                    REFERENCE_SEQUENCE.getAbsolutePath());
         }
 
         final Set<String> samples = new HashSet<>();
diff --git a/src/main/java/picard/analysis/CollectWgsMetrics.java b/src/main/java/picard/analysis/CollectWgsMetrics.java
@@ -48,6 +48,7 @@
 import org.broadinstitute.barclay.argparser.ArgumentCollection;
 import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
 import org.broadinstitute.barclay.help.DocumentedFeature;
+import picard.PicardException;
 import picard.cmdline.CommandLineProgram;
 import picard.cmdline.StandardOptionDefinitions;
 import picard.cmdline.argumentcollections.IntervalArgumentCollection;
@@ -57,6 +58,7 @@
 import picard.filter.CountingFilter;
 import picard.filter.CountingMapQFilter;
 import picard.filter.CountingPairedFilter;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.File;
 import java.util.ArrayList;
@@ -211,7 +213,11 @@ protected int doWork() {
 
         // Verify the sequence dictionaries match
         if (!this.header.getSequenceDictionary().isEmpty()) {
-            SequenceUtil.assertSequenceDictionariesEqual(this.header.getSequenceDictionary(), refWalker.getSequenceDictionary());
+            SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                    this.header.getSequenceDictionary(),
+                    INPUT.getAbsolutePath(),
+                    refWalker.getSequenceDictionary(),
+                    REFERENCE_SEQUENCE.getAbsolutePath());
         }
 
         final List<SamRecordFilter> filters = new ArrayList<>();
diff --git a/src/main/java/picard/analysis/SinglePassSamProgram.java b/src/main/java/picard/analysis/SinglePassSamProgram.java
@@ -35,14 +35,14 @@
 import htsjdk.samtools.util.IOUtil;
 import htsjdk.samtools.util.Log;
 import htsjdk.samtools.util.ProgressLogger;
-import htsjdk.samtools.util.SequenceUtil;
 import org.broadinstitute.barclay.argparser.Argument;
 import org.broadinstitute.barclay.argparser.ArgumentCollection;
 import picard.PicardException;
 import picard.cmdline.CommandLineProgram;
 import picard.cmdline.StandardOptionDefinitions;
 import picard.cmdline.argumentcollections.OutputArgumentCollection;
 import picard.cmdline.argumentcollections.RequiredOutputArgumentCollection;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.File;
 import java.util.Arrays;
@@ -114,8 +114,11 @@ public static void makeItSo(final File input,
             walker = new ReferenceSequenceFileWalker(referenceSequence);
 
             if (!in.getFileHeader().getSequenceDictionary().isEmpty()) {
-                SequenceUtil.assertSequenceDictionariesEqual(in.getFileHeader().getSequenceDictionary(),
-                        walker.getSequenceDictionary());
+                SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                        in.getFileHeader().getSequenceDictionary(),
+                        input.getAbsolutePath(),
+                        walker.getSequenceDictionary(),
+                        referenceSequence.getAbsolutePath());
             }
         }
 
diff --git a/src/main/java/picard/analysis/directed/CollectTargetedMetrics.java b/src/main/java/picard/analysis/directed/CollectTargetedMetrics.java
@@ -15,12 +15,14 @@
 import htsjdk.samtools.util.ProgressLogger;
 import htsjdk.samtools.util.SequenceUtil;
 import org.broadinstitute.barclay.argparser.Argument;
+import picard.PicardException;
 import picard.analysis.MetricAccumulationLevel;
 import picard.analysis.TheoreticalSensitivity;
 import picard.analysis.TheoreticalSensitivityMetrics;
 import picard.cmdline.CommandLineProgram;
 import picard.cmdline.StandardOptionDefinitions;
 import picard.metrics.MultilevelMetrics;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.File;
 import java.util.*;
@@ -121,22 +123,26 @@ protected int doWork() {
         final IntervalList targetIntervals = IntervalList.fromFiles(TARGET_INTERVALS);
 
         // Validate that the targets and baits have the same references as the reads file
-        SequenceUtil.assertSequenceDictionariesEqual(
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
                 reader.getFileHeader().getSequenceDictionary(),
-                targetIntervals.getHeader().getSequenceDictionary());
-        SequenceUtil.assertSequenceDictionariesEqual(
+                INPUT.getAbsolutePath(),
+                targetIntervals.getHeader().getSequenceDictionary(),
+                "target intervals");
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
                 reader.getFileHeader().getSequenceDictionary(),
-                getProbeIntervals().getHeader().getSequenceDictionary()
-        );
+                INPUT.getAbsolutePath(),
+                getProbeIntervals().getHeader().getSequenceDictionary(),
+                "probe intervals");
 
         ReferenceSequenceFile ref = null;
         if (REFERENCE_SEQUENCE != null) {
             IOUtil.assertFileIsReadable(REFERENCE_SEQUENCE);
             ref = ReferenceSequenceFileFactory.getReferenceSequenceFile(REFERENCE_SEQUENCE);
-            SequenceUtil.assertSequenceDictionariesEqual(
-                    reader.getFileHeader().getSequenceDictionary(), ref.getSequenceDictionary(),
-                    INPUT, REFERENCE_SEQUENCE
-            );
+            SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                    reader.getFileHeader().getSequenceDictionary(),
+                    INPUT.getAbsolutePath(),
+                    ref.getSequenceDictionary(),
+                    REFERENCE_SEQUENCE.getAbsolutePath());
         }
 
         final COLLECTOR collector = makeCollector(
diff --git a/src/main/java/picard/analysis/replicates/CollectIndependentReplicateMetrics.java b/src/main/java/picard/analysis/replicates/CollectIndependentReplicateMetrics.java
@@ -45,7 +45,6 @@
 import htsjdk.samtools.util.IOUtil;
 import htsjdk.samtools.util.Log;
 import htsjdk.samtools.util.ProgressLogger;
-import htsjdk.samtools.util.SequenceUtil;
 import htsjdk.utils.ValidationUtils;
 import htsjdk.variant.variantcontext.Allele;
 import htsjdk.variant.variantcontext.VariantContext;
@@ -67,6 +66,7 @@
 import picard.cmdline.StandardOptionDefinitions;
 import picard.cmdline.programgroups.DiagnosticsAndQCProgramGroup;
 import picard.filter.CountingPairedFilter;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.File;
 import java.util.HashMap;
@@ -207,8 +207,11 @@ protected int doWork() {
         final VCFFileReader vcf = new VCFFileReader(VCF, false);
         final VCFHeader vcfFileHeader = vcf.getFileHeader();
 
-
-        SequenceUtil.assertSequenceDictionariesEqual(in.getFileHeader().getSequenceDictionary(), vcfFileHeader.getSequenceDictionary());
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                in.getFileHeader().getSequenceDictionary(),
+                INPUT.getAbsolutePath(),
+                vcfFileHeader.getSequenceDictionary(),
+                VCF.getAbsolutePath());
 
         final List<String> samples = vcfFileHeader.getSampleNamesInOrder();
 
diff --git a/src/main/java/picard/fingerprint/CheckFingerprint.java b/src/main/java/picard/fingerprint/CheckFingerprint.java
@@ -248,9 +248,16 @@ protected int doWork() {
         final FingerprintChecker checker = new FingerprintChecker(HAPLOTYPE_MAP);
         checker.setReferenceFasta(REFERENCE_SEQUENCE);
 
-        SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), SAMSequenceDictionaryExtractor.extractDictionary(genotypesPath), true);
-        SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), checker.getHeader().getSequenceDictionary(), true);
-
+        try {
+            SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), SAMSequenceDictionaryExtractor.extractDictionary(genotypesPath), true);
+        } catch (final SequenceUtil.SequenceListsDifferException e) {
+            throw new PicardException("Dictionary in " + inputPath + " does not match dictionary in " + genotypesPath, e);
+        }
+        try {
+            SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), checker.getHeader().getSequenceDictionary(), true);
+        } catch (final SequenceUtil.SequenceListsDifferException e) {
+            throw new PicardException("Dictionary in " + inputPath + " does not match dictionary in " + HAPLOTYPE_MAP, e);
+        }
 
 
         final String observedSampleAlias = extractObservedSampleName(inputPath, OBSERVED_SAMPLE_ALIAS);
diff --git a/src/main/java/picard/fingerprint/FingerprintChecker.java b/src/main/java/picard/fingerprint/FingerprintChecker.java
@@ -223,9 +223,13 @@ private void checkDictionaryGoodForFingerprinting(final SAMSequenceDictionary se
         final SAMSequenceDictionary activeDictionary = getActiveDictionary(haplotypes);
 
         if (sequenceDictionaryToCheck.getSequences().size() < activeDictionary.size()) {
-            throw new IllegalArgumentException("Dictionary on fingerprinted file smaller than that on Haplotype Database!");
+            throw new SequenceUtil.SequenceListsDifferException("Dictionary on fingerprinted file smaller than that on Haplotype Database!");
+        }
+        try {
+            SequenceUtil.assertSequenceDictionariesEqual(activeDictionary, sequenceDictionaryToCheck, true);
+        } catch (final SequenceUtil.SequenceListsDifferException e) {
+            throw new PicardException("Dictionary on fingerprinted file does not match dictionary in Haplotype Database.", e);
         }
-        SequenceUtil.assertSequenceDictionariesEqual(activeDictionary, sequenceDictionaryToCheck, true);
     }
 
     private static SAMSequenceDictionary getActiveDictionary(final HaplotypeMap haplotypes) {
diff --git a/src/main/java/picard/reference/ExtractSequences.java b/src/main/java/picard/reference/ExtractSequences.java
@@ -39,6 +39,7 @@
 import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
 import picard.cmdline.programgroups.ReferenceProgramGroup;
 import picard.cmdline.StandardOptionDefinitions;
+import picard.util.SequenceDictionaryUtils;
 
 import java.io.BufferedWriter;
 import java.io.File;
@@ -93,7 +94,11 @@ protected int doWork() {
 
         final IntervalList intervals = IntervalList.fromFile(INTERVAL_LIST);
         final ReferenceSequenceFile ref = ReferenceSequenceFileFactory.getReferenceSequenceFile(REFERENCE_SEQUENCE);
-        SequenceUtil.assertSequenceDictionariesEqual(intervals.getHeader().getSequenceDictionary(), ref.getSequenceDictionary());
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                intervals.getHeader().getSequenceDictionary(),
+                INTERVAL_LIST.getAbsolutePath(),
+                ref.getSequenceDictionary(),
+                REFERENCE_SEQUENCE.getAbsolutePath());
 
         final BufferedWriter out = IOUtil.openFileForBufferedWriting(OUTPUT);
 
diff --git a/src/main/java/picard/util/BaitDesigner.java b/src/main/java/picard/util/BaitDesigner.java
@@ -450,8 +450,11 @@ protected int doWork() {
         final ReferenceSequenceFileWalker referenceWalker = new ReferenceSequenceFileWalker(REFERENCE_SEQUENCE);
 
         // Check that the reference and the target list have matching headers
-        SequenceUtil.assertSequenceDictionariesEqual(referenceWalker.getSequenceDictionary(),
-                targets.getHeader().getSequenceDictionary());
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                referenceWalker.getSequenceDictionary(),
+                REFERENCE_SEQUENCE.getAbsolutePath(),
+                targets.getHeader().getSequenceDictionary(),
+                TARGETS.getAbsolutePath());
 
         // Design the baits!
         int discardedBaits = 0;
diff --git a/src/main/java/picard/util/SequenceDictionaryUtils.java b/src/main/java/picard/util/SequenceDictionaryUtils.java
@@ -23,6 +23,7 @@
  */
 package picard.util;
 
+import htsjdk.samtools.SAMSequenceDictionary;
 import htsjdk.samtools.SAMSequenceDictionaryCodec;
 import htsjdk.samtools.SAMSequenceRecord;
 import htsjdk.samtools.reference.ReferenceSequence;
@@ -32,6 +33,7 @@
 import htsjdk.samtools.util.IOUtil;
 import htsjdk.samtools.util.RuntimeIOException;
 import htsjdk.samtools.util.SortingCollection;
+import htsjdk.samtools.util.SequenceUtil;
 import htsjdk.samtools.util.StringUtil;
 import picard.PicardException;
 
@@ -192,6 +194,30 @@ public static SortingCollection<String> makeSortingCollection() {
         );
     }
 
+    /**
+     * Throw an exception if the two provided sequence dictionaries are not equal.
+     *
+     * @param firstDict first dictionary to compare
+     * @param firstDictSource a user-recognizable message identifying the source of the first dictionary, preferably a file path
+     * @param secondDict second dictionary to compare
+     * @param secondDictSource a user-recognizable message identifying the source of the second dictionary,  preferably a file path
+     */
+    public static void assertSequenceDictionariesEqual(
+            final SAMSequenceDictionary firstDict,
+            final String firstDictSource,
+            final SAMSequenceDictionary secondDict,
+            final String secondDictSource) {
+        try {
+            SequenceUtil.assertSequenceDictionariesEqual(firstDict, secondDict);
+        } catch (final SequenceUtil.SequenceListsDifferException e) {
+            throw new PicardException(
+                    String.format("Sequence dictionary for (%s) does not match sequence dictionary for (%s)",
+                            firstDictSource,
+                            secondDictSource),
+                    e);
+        }
+    }
+
     private static class StringCodec implements SortingCollection.Codec<String> {
         private DataInputStream dis;
         private DataOutputStream dos;
diff --git a/src/main/java/picard/vcf/GenotypeConcordance.java b/src/main/java/picard/vcf/GenotypeConcordance.java
@@ -31,7 +31,6 @@
 import htsjdk.samtools.util.IntervalList;
 import htsjdk.samtools.util.Log;
 import htsjdk.samtools.util.ProgressLogger;
-import htsjdk.samtools.util.SequenceUtil;
 import htsjdk.tribble.Tribble;
 import htsjdk.variant.variantcontext.Allele;
 import htsjdk.variant.variantcontext.Genotype;
@@ -56,6 +55,7 @@
 import picard.cmdline.StandardOptionDefinitions;
 import picard.cmdline.programgroups.VariantEvaluationProgramGroup;
 import picard.nio.PicardHtsPath;
+import picard.util.SequenceDictionaryUtils;
 import picard.vcf.GenotypeConcordanceStates.CallState;
 import picard.vcf.GenotypeConcordanceStates.ContingencyState;
 import picard.vcf.GenotypeConcordanceStates.TruthAndCallStates;
@@ -381,13 +381,21 @@ private boolean indexExists(final Path vcf) {
         }
 
         // Verify that both VCFs have the same Sequence Dictionary
-        SequenceUtil.assertSequenceDictionariesEqual(truthReader.getFileHeader().getSequenceDictionary(), callReader.getFileHeader().getSequenceDictionary());
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                truthReader.getFileHeader().getSequenceDictionary(),
+                TRUTH_VCF.getRawInputString(),
+                callReader.getFileHeader().getSequenceDictionary(),
+                CALL_VCF.getRawInputString());
 
         final Optional<VariantContextWriter> writer = getVariantContextWriter(truthReader, callReader);
 
         if (usingIntervals) {
             // If using intervals, verify that the sequence dictionaries agree with those of the VCFs
-            SequenceUtil.assertSequenceDictionariesEqual(intervalsSamSequenceDictionary, truthReader.getFileHeader().getSequenceDictionary());
+            SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                    intervalsSamSequenceDictionary,
+                    "provided intervals",
+                    truthReader.getFileHeader().getSequenceDictionary(),
+                    TRUTH_VCF.getRawInputString());
         }
 
         // Build the pair of iterators over the regions of interest
diff --git a/src/test/java/picard/analysis/CollectWgsMetricsTest.java b/src/test/java/picard/analysis/CollectWgsMetricsTest.java
@@ -40,6 +40,7 @@
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
+import picard.PicardException;
 import picard.cmdline.CommandLineProgramTest;
 import picard.sam.SortSam;
 import picard.util.TestNGUtil;
@@ -530,7 +531,7 @@ public void testAdapterReads(final String useFastAlgorithm) throws IOException {
         }
     }
 
-    @Test(expectedExceptions = SequenceUtil.SequenceListsDifferException.class)
+    @Test(expectedExceptions = PicardException.class)
     public void testFailDifferentSequenceDictionaries() throws IOException {
         final File input = new File(TEST_DIR, "forMetrics.sam");
         final File outfile = getTempOutputFile("test", ".wgs_metrics");
diff --git a/src/test/java/picard/util/SequenceDictionaryUtilsTest.java b/src/test/java/picard/util/SequenceDictionaryUtilsTest.java
@@ -0,0 +1,48 @@
+package picard.util;
+
+import htsjdk.samtools.SAMSequenceDictionary;
+import htsjdk.samtools.SAMSequenceRecord;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import picard.PicardException;
+
+import java.util.List;
+
+public class SequenceDictionaryUtilsTest {
+
+    @Test
+    public void testSequenceDictionaryPositive() {
+        final SAMSequenceDictionary s1 = new SAMSequenceDictionary(
+                List.of(
+                        new SAMSequenceRecord("chr1", 10),
+                        new SAMSequenceRecord("chr2", 15),
+                        new SAMSequenceRecord("chr3", 20)
+                )
+        );
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(
+                s1,
+                "s1",
+                new SAMSequenceDictionary(s1.getSequences()),
+                "s2");
+    }
+
+    @DataProvider
+    public Object[][] sequenceDictionaryNegativeProvider() {
+        return new Object[][]{
+                {
+                        new SAMSequenceDictionary(List.of(new SAMSequenceRecord("chr1", 100))),
+                        new SAMSequenceDictionary(List.of(new SAMSequenceRecord("chr1", 200)))
+                },
+                {
+                        new SAMSequenceDictionary(List.of(new SAMSequenceRecord("chr1", 10))),
+                        new SAMSequenceDictionary(List.of(new SAMSequenceRecord("chr2", 10)))
+                },
+        };
+    }
+
+    @Test(dataProvider = "sequenceDictionaryNegativeProvider", expectedExceptions = PicardException.class)
+    public void testSequenceDictionaryNegative(final SAMSequenceDictionary s1, final SAMSequenceDictionary s2 ) {
+        SequenceDictionaryUtils.assertSequenceDictionariesEqual(s1, "s1", s2, "s2");
+    }
+
+}

Original file line number	Diff line number	Diff line change
`@@ -223,9 +223,13 @@ private void checkDictionaryGoodForFingerprinting(final SAMSequenceDictionary se`
`223`	`223`	`final SAMSequenceDictionary activeDictionary = getActiveDictionary(haplotypes);`
`224`	`224`
`225`	`225`	`if (sequenceDictionaryToCheck.getSequences().size() < activeDictionary.size()) {`
`226`		`- throw new IllegalArgumentException("Dictionary on fingerprinted file smaller than that on Haplotype Database!");`
	`226`	`+ throw new SequenceUtil.SequenceListsDifferException("Dictionary on fingerprinted file smaller than that on Haplotype Database!");`
	`227`	`+ }`
	`228`	`+ try {`
	`229`	`+ SequenceUtil.assertSequenceDictionariesEqual(activeDictionary, sequenceDictionaryToCheck, true);`
	`230`	`+ } catch (final SequenceUtil.SequenceListsDifferException e) {`
	`231`	`+ throw new PicardException("Dictionary on fingerprinted file does not match dictionary in Haplotype Database.", e);`
`227`	`232`	`}`
`228`		`- SequenceUtil.assertSequenceDictionariesEqual(activeDictionary, sequenceDictionaryToCheck, true);`
`229`	`233`	`}`
`230`	`234`
`231`	`235`	`private static SAMSequenceDictionary getActiveDictionary(final HaplotypeMap haplotypes) {`