From 0629d956f666d6643a9c4d9b00c3d0abbc77e12a Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Thu, 13 Apr 2023 10:54:52 -0400 Subject: [PATCH 1/6] Create a (mostly) empty metrics file instead of giving a stacktrace if no reads pass Illumina filters. --- .../java/picard/analysis/AlignmentSummaryMetricsCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java index 530b66c50e..6710c2c491 100644 --- a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java +++ b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java @@ -261,7 +261,7 @@ public void acceptRecord(final SAMRecordAndReference samRecordAndReference) { @Override public void finish() { //summarize read data - if (metrics.TOTAL_READS > 0) { + if (metrics.TOTAL_READS > 0 && metrics.PF_READS > 0) { metrics.PCT_PF_READS = (double) metrics.PF_READS / (double) metrics.TOTAL_READS; metrics.PCT_ADAPTER = adapterReads / (double) metrics.PF_READS; metrics.MEAN_READ_LENGTH = readLengthHistogram.getMean(); From b24e51220a4af72f0a3ea27140d1d1b835fe5c04 Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Fri, 14 Apr 2023 12:20:13 -0400 Subject: [PATCH 2/6] Added test for the PF_READS == 0 case. The new check should make checking if TOTAL_READS > 0 redundant, so that's removed now. --- .../AlignmentSummaryMetricsCollector.java | 2 +- .../CollectAlignmentSummaryMetricsTest.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java index 6710c2c491..7fe76e60d1 100644 --- a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java +++ b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java @@ -261,7 +261,7 @@ public void acceptRecord(final SAMRecordAndReference samRecordAndReference) { @Override public void finish() { //summarize read data - if (metrics.TOTAL_READS > 0 && metrics.PF_READS > 0) { + if (metrics.PF_READS > 0) { metrics.PCT_PF_READS = (double) metrics.PF_READS / (double) metrics.TOTAL_READS; metrics.PCT_ADAPTER = adapterReads / (double) metrics.PF_READS; metrics.MEAN_READ_LENGTH = readLengthHistogram.getMean(); diff --git a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java index 78cb4d1320..5987fb7d6d 100644 --- a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java +++ b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java @@ -768,4 +768,37 @@ public void testReadLengthHistogram(final boolean plotChart) throws IOException } } } + + @Test + public void testNoPFReads() throws IOException { + final File input = new File(TEST_DATA_DIR, "null.sam"); + final File outfile = getTempOutputFile("test", ".txt"); + final String[] args = new String[]{ + "INPUT=" + input.getAbsolutePath(), + "OUTPUT=" + outfile.getAbsolutePath(), + }; + Assert.assertEquals(runPicardCommandLine(args), 0); + + final MetricsFile> output = new MetricsFile<>(); + try (FileReader reader = new FileReader(outfile)) { + output.read(reader); + } + + Assert.assertEquals(output.getMetrics().size(), 1); + for (final AlignmentSummaryMetrics metrics : output.getMetrics()) { + Assert.assertEquals(metrics.MEAN_READ_LENGTH, 0.0); + Assert.assertEquals(metrics.TOTAL_READS, 3); + Assert.assertEquals(metrics.PF_READS, 0); + Assert.assertEquals(metrics.PF_NOISE_READS, 0); + Assert.assertEquals(metrics.PF_HQ_ALIGNED_READS, 0); + Assert.assertEquals(metrics.PF_HQ_ALIGNED_Q20_BASES, 0); + Assert.assertEquals(metrics.PF_HQ_MEDIAN_MISMATCHES, 0.0); + Assert.assertEquals(metrics.PF_READS_ALIGNED, 0); + Assert.assertEquals(metrics.PF_READS_IMPROPER_PAIRS, 0); + Assert.assertEquals(metrics.PCT_PF_READS_IMPROPER_PAIRS, 0.0); + Assert.assertEquals(metrics.PF_ALIGNED_BASES, 0); + Assert.assertEquals(metrics.PF_MISMATCH_RATE, 0.0); + Assert.assertEquals(metrics.BAD_CYCLES, 0); + } + } } From 3e72e7b7657e22b3b0a8c4d32db880de9132a254 Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Fri, 14 Apr 2023 12:21:03 -0400 Subject: [PATCH 3/6] Added the test file --- testdata/picard/sam/AlignmentSummaryMetrics/null.sam | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 testdata/picard/sam/AlignmentSummaryMetrics/null.sam diff --git a/testdata/picard/sam/AlignmentSummaryMetrics/null.sam b/testdata/picard/sam/AlignmentSummaryMetrics/null.sam new file mode 100644 index 0000000000..06d649d2ea --- /dev/null +++ b/testdata/picard/sam/AlignmentSummaryMetrics/null.sam @@ -0,0 +1,5 @@ +@HD VN:1.6 GO:none SO:queryname +@RG ID:test SM:test_sample LB:test_lib PL:ILLUMINA PU:my_platform CN:BI DT:2023-01-01T00:00:00-0500 DS:description +aln1 516 * 0 0 * * 0 0 AGCTACTG 99999999 RG:Z:test +aln2 516 * 0 0 * * 0 0 GTCAGTCA 99999999 RG:Z:test +aln3 516 * 0 0 * * 0 0 CCTTGGAA 99999999 RG:Z:test From 933a60541f4f830d6dfd19ab21f33e1b930cedc6 Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Tue, 9 May 2023 14:39:40 -0400 Subject: [PATCH 4/6] Use a PicardException with an informative message instead. --- .../java/picard/analysis/AlignmentSummaryMetricsCollector.java | 3 +++ .../picard/analysis/CollectAlignmentSummaryMetricsTest.java | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java index 7fe76e60d1..47e6bab0b5 100644 --- a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java +++ b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java @@ -40,6 +40,7 @@ import htsjdk.samtools.util.CoordMath; import htsjdk.samtools.util.Histogram; import htsjdk.samtools.util.SequenceUtil; +import picard.PicardException; import picard.metrics.PerUnitMetricCollector; import picard.metrics.SAMRecordAndReference; import picard.metrics.SAMRecordAndReferenceMultiLevelCollector; @@ -298,6 +299,8 @@ public void finish() { metrics.PF_HQ_MEDIAN_MISMATCHES = hqMismatchHistogram.getMedian(); } + } else { + throw new PicardException("Input file contains no PF_READS."); } } diff --git a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java index 5987fb7d6d..0bfcbccae8 100644 --- a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java +++ b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java @@ -29,6 +29,7 @@ import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import picard.PicardException; import picard.cmdline.CommandLineProgramTest; import picard.util.TestNGUtil; @@ -769,7 +770,7 @@ public void testReadLengthHistogram(final boolean plotChart) throws IOException } } - @Test + @Test(expectedExceptions = PicardException.class) public void testNoPFReads() throws IOException { final File input = new File(TEST_DATA_DIR, "null.sam"); final File outfile = getTempOutputFile("test", ".txt"); From b80aa0c3b0655307d7c7f7a1fc2fe53008916176 Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Tue, 9 May 2023 16:30:17 -0400 Subject: [PATCH 5/6] Reverting latest changes to check why some tests fail. --- .../java/picard/analysis/AlignmentSummaryMetricsCollector.java | 3 --- .../picard/analysis/CollectAlignmentSummaryMetricsTest.java | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java index 47e6bab0b5..7fe76e60d1 100644 --- a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java +++ b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java @@ -40,7 +40,6 @@ import htsjdk.samtools.util.CoordMath; import htsjdk.samtools.util.Histogram; import htsjdk.samtools.util.SequenceUtil; -import picard.PicardException; import picard.metrics.PerUnitMetricCollector; import picard.metrics.SAMRecordAndReference; import picard.metrics.SAMRecordAndReferenceMultiLevelCollector; @@ -299,8 +298,6 @@ public void finish() { metrics.PF_HQ_MEDIAN_MISMATCHES = hqMismatchHistogram.getMedian(); } - } else { - throw new PicardException("Input file contains no PF_READS."); } } diff --git a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java index 0bfcbccae8..5987fb7d6d 100644 --- a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java +++ b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java @@ -29,7 +29,6 @@ import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; -import picard.PicardException; import picard.cmdline.CommandLineProgramTest; import picard.util.TestNGUtil; @@ -770,7 +769,7 @@ public void testReadLengthHistogram(final boolean plotChart) throws IOException } } - @Test(expectedExceptions = PicardException.class) + @Test public void testNoPFReads() throws IOException { final File input = new File(TEST_DATA_DIR, "null.sam"); final File outfile = getTempOutputFile("test", ".txt"); From 4b92bbc94a791d27fba3f1dc880af2fc47e01753 Mon Sep 17 00:00:00 2001 From: Can Kockan Date: Tue, 9 May 2023 16:57:43 -0400 Subject: [PATCH 6/6] Add another check to make sure TOTAL_READS > 0 before throwing a PicardException for PF_READS == 0 --- .../picard/analysis/AlignmentSummaryMetricsCollector.java | 5 +++++ .../picard/analysis/CollectAlignmentSummaryMetricsTest.java | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java index 7fe76e60d1..70ff3b91d2 100644 --- a/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java +++ b/src/main/java/picard/analysis/AlignmentSummaryMetricsCollector.java @@ -40,6 +40,7 @@ import htsjdk.samtools.util.CoordMath; import htsjdk.samtools.util.Histogram; import htsjdk.samtools.util.SequenceUtil; +import picard.PicardException; import picard.metrics.PerUnitMetricCollector; import picard.metrics.SAMRecordAndReference; import picard.metrics.SAMRecordAndReferenceMultiLevelCollector; @@ -298,6 +299,10 @@ public void finish() { metrics.PF_HQ_MEDIAN_MISMATCHES = hqMismatchHistogram.getMedian(); } + } else { + if (metrics.TOTAL_READS > 0) { + throw new PicardException("Input file contains no PF_READS."); + } } } diff --git a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java index 5987fb7d6d..0bfcbccae8 100644 --- a/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java +++ b/src/test/java/picard/analysis/CollectAlignmentSummaryMetricsTest.java @@ -29,6 +29,7 @@ import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import picard.PicardException; import picard.cmdline.CommandLineProgramTest; import picard.util.TestNGUtil; @@ -769,7 +770,7 @@ public void testReadLengthHistogram(final boolean plotChart) throws IOException } } - @Test + @Test(expectedExceptions = PicardException.class) public void testNoPFReads() throws IOException { final File input = new File(TEST_DATA_DIR, "null.sam"); final File outfile = getTempOutputFile("test", ".txt");