From ec16439387c88c1d7698f3e9f9ab2cd7e24b71ae Mon Sep 17 00:00:00 2001 From: Joel Thibault Date: Thu, 13 Feb 2014 16:23:47 -0500 Subject: [PATCH 1/3] Clear the ReadCovariates keysCache before runs of individual Unit Tests - normal runs have a constant covariate count, so this is not necessary --- .../sting/utils/recalibration/ReadCovariates.java | 8 ++++++++ .../gatk/walkers/bqsr/ReadRecalibrationInfoUnitTest.java | 7 ++++++- .../utils/recalibration/ContextCovariateUnitTest.java | 5 +++++ .../sting/utils/recalibration/CycleCovariateUnitTest.java | 6 ++++++ .../sting/utils/recalibration/ReadCovariatesUnitTest.java | 8 +++++++- .../utils/recalibration/ReadGroupCovariateUnitTest.java | 6 ++++++ .../utils/recalibration/RecalibrationReportUnitTest.java | 6 ++++++ .../utils/recalibration/RepeatCovariatesUnitTest.java | 6 ++++++ 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java b/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java index 6cbbbd089..9ef94b8e8 100644 --- a/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java +++ b/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java @@ -78,6 +78,14 @@ public class ReadCovariates { } }; + /** + * The keys cache is only valid for a single covariate count. Normally this will remain constant for the analysis. + * If running multiple analyses (or the unit test suite), it's necessary to clear the cache. + */ + public static void clearKeysCache() { + keysCache.remove(); + } + /** * Our keys, indexed by event type x read length x covariate */ diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/gatk/walkers/bqsr/ReadRecalibrationInfoUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/gatk/walkers/bqsr/ReadRecalibrationInfoUnitTest.java index 12fa2525f..39cf719dd 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/gatk/walkers/bqsr/ReadRecalibrationInfoUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/gatk/walkers/bqsr/ReadRecalibrationInfoUnitTest.java @@ -53,6 +53,7 @@ import org.broadinstitute.sting.utils.recalibration.ReadCovariates; import org.broadinstitute.sting.utils.sam.ArtificialSAMUtils; import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -62,6 +63,11 @@ import java.util.EnumMap; import java.util.List; public final class ReadRecalibrationInfoUnitTest extends BaseTest { + @BeforeMethod + public void init() { + ReadCovariates.clearKeysCache(); + } + @DataProvider(name = "InfoProvider") public Object[][] createCombineTablesProvider() { List tests = new ArrayList(); @@ -74,7 +80,6 @@ public final class ReadRecalibrationInfoUnitTest extends BaseTest { return tests.toArray(new Object[][]{}); } - @Test(dataProvider = "InfoProvider") public void testReadInfo(final int readLength, final boolean includeIndelErrors) { final ReadCovariates covariates = new ReadCovariates(readLength, 2); diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ContextCovariateUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ContextCovariateUnitTest.java index 2d3d680df..7e2581c51 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ContextCovariateUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ContextCovariateUnitTest.java @@ -55,6 +55,7 @@ import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.broadinstitute.sting.utils.sam.ReadUtils; import org.testng.Assert; import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; /** @@ -70,7 +71,11 @@ public class ContextCovariateUnitTest { RAC = new RecalibrationArgumentCollection(); covariate = new ContextCovariate(); covariate.initialize(RAC); + } + @BeforeMethod + public void initCache() { + ReadCovariates.clearKeysCache(); } @Test(enabled = true) diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/CycleCovariateUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/CycleCovariateUnitTest.java index ce827065b..4f8a70cc9 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/CycleCovariateUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/CycleCovariateUnitTest.java @@ -54,6 +54,7 @@ import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.broadinstitute.sting.utils.sam.ReadUtils; import org.testng.Assert; import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; /** @@ -71,6 +72,11 @@ public class CycleCovariateUnitTest { covariate.initialize(RAC); } + @BeforeMethod + public void initCache() { + ReadCovariates.clearKeysCache(); + } + @Test(enabled = true) public void testSimpleCycles() { short readLength = 10; diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadCovariatesUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadCovariatesUnitTest.java index f20d6116b..eea8aa8f3 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadCovariatesUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadCovariatesUnitTest.java @@ -53,6 +53,7 @@ import org.broadinstitute.sting.utils.sam.GATKSAMReadGroupRecord; import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.broadinstitute.sting.utils.sam.ReadUtils; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.Random; @@ -63,9 +64,14 @@ import java.util.Random; */ public class ReadCovariatesUnitTest { + @BeforeMethod + public void init() { + ReadCovariates.clearKeysCache(); + } + @Test(enabled = false) public void testCovariateGeneration() { - final RecalibrationArgumentCollection RAC = new RecalibrationArgumentCollection(); + final RecalibrationArgumentCollection RAC = new RecalibrationArgumentCollection(); final String RGID = "id"; ReadGroupCovariate rgCov = new ReadGroupCovariate(); diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadGroupCovariateUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadGroupCovariateUnitTest.java index 0b2df6369..a8366ce5c 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadGroupCovariateUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/ReadGroupCovariateUnitTest.java @@ -53,6 +53,7 @@ import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.broadinstitute.sting.utils.sam.ReadUtils; import org.testng.Assert; import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; /** @@ -70,6 +71,11 @@ public class ReadGroupCovariateUnitTest { covariate.initialize(RAC); } + @BeforeMethod + public void initCache() { + ReadCovariates.clearKeysCache(); + } + @Test(enabled = true) public void testSingleRecord() { final String expected = "SAMPLE.1"; diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java index f382fc116..256cbbea4 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java @@ -54,6 +54,7 @@ import org.broadinstitute.sting.utils.sam.GATKSAMReadGroupRecord; import org.broadinstitute.sting.utils.sam.GATKSAMRecord; import org.broadinstitute.sting.utils.sam.ReadUtils; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.*; @@ -63,6 +64,11 @@ import java.util.*; * @since 4/21/12 */ public class RecalibrationReportUnitTest { + @BeforeMethod + public void init() { + ReadCovariates.clearKeysCache(); + } + private static RecalDatum createRandomRecalDatum(int maxObservations, int maxErrors) { final Random random = new Random(); final int nObservations = random.nextInt(maxObservations); diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java index 180bdd3c7..85c8e3ea1 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java @@ -56,6 +56,7 @@ import org.broadinstitute.sting.utils.BaseUtils; import org.broadinstitute.sting.utils.collections.Pair; import org.testng.Assert; import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.ArrayList; @@ -82,6 +83,11 @@ public class RepeatCovariatesUnitTest { rurlCovariate.initialize(RAC); } + @BeforeMethod + public void initCache() { + ReadCovariates.clearKeysCache(); + } + @Test(enabled = true) public void testFindNumberOfRepetitions() { From c8a5007c85988db81f3980c5def5ef3593aa1632 Mon Sep 17 00:00:00 2001 From: Joel Thibault Date: Fri, 14 Feb 2014 11:40:22 -0500 Subject: [PATCH 2/3] Add a comment to the method where the error appears --- .../sting/utils/recalibration/ReadCovariates.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java b/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java index 9ef94b8e8..6efed2689 100644 --- a/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java +++ b/protected/gatk-protected/src/main/java/org/broadinstitute/sting/utils/recalibration/ReadCovariates.java @@ -116,6 +116,10 @@ public class ReadCovariates { /** * Update the keys for mismatch, insertion, and deletion for the current covariate at read offset * + * NOTE: no checks are performed on the number of covariates, for performance reasons. If the count increases + * after the keysCache has been accessed, this method will throw an ArrayIndexOutOfBoundsException. This currently + * only occurs in the testing harness, and we don't anticipate that it will become a part of normal runs. + * * @param mismatch the mismatch key value * @param insertion the insertion key value * @param deletion the deletion key value From cb7ad01202615ffcf8799e6671b6eca64cfbe440 Mon Sep 17 00:00:00 2001 From: Joel Thibault Date: Fri, 14 Feb 2014 12:34:08 -0500 Subject: [PATCH 3/3] Re-enable the relevant tests --- .../utils/recalibration/RecalibrationReportUnitTest.java | 2 +- .../sting/utils/recalibration/RepeatCovariatesUnitTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java index 256cbbea4..d3c3ffe97 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RecalibrationReportUnitTest.java @@ -77,7 +77,7 @@ public class RecalibrationReportUnitTest { return new RecalDatum((long)nObservations, (double)nErrors, (byte)qual); } - @Test(enabled = false) + @Test public void testOutput() { final int length = 100; diff --git a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java index 85c8e3ea1..74cb2a1eb 100644 --- a/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java +++ b/protected/gatk-protected/src/test/java/org/broadinstitute/sting/utils/recalibration/RepeatCovariatesUnitTest.java @@ -89,7 +89,7 @@ public class RepeatCovariatesUnitTest { } - @Test(enabled = true) + @Test public void testFindNumberOfRepetitions() { // First, test logic to compute number of repetitions of a substring on a given string. int result = GATKVariantContextUtils.findNumberofRepetitions("AC".getBytes(), "ACAC".getBytes(), true); @@ -142,7 +142,7 @@ public class RepeatCovariatesUnitTest { * Build synthetic reads with random content made up of tandem repeats, record computed Repeat Unit and # repeats and see if * they match with read context */ - @Test(enabled = false) + @Test public void testManyObservations() { final int NUM_UNITS = 10; final int MAX_REPEAT_UNIT_LENGTH = RAC.MAX_STR_UNIT_LENGTH;