From 7cab6f6bb0332a523877bf3ad5fd3967d2024727 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Thu, 13 Oct 2011 15:53:12 -0400 Subject: [PATCH 01/20] Bug fixes for thread unsafe simple timer and bad Ns treatment in AlignmentUtils -- SimpleTimer is now threadsafe using synchronized method keywords -- Bug fix for alignmentToByteArray() where the N case was refPos++ not the now correct refPos += elementLength --- .../src/org/broadinstitute/sting/utils/SimpleTimer.java | 9 ++++----- .../broadinstitute/sting/utils/sam/AlignmentUtils.java | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java b/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java index 342087b41..6d228640f 100644 --- a/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java +++ b/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java @@ -58,7 +58,7 @@ public class SimpleTimer { */ @Requires("running == false") @Ensures({"result != null", "elapsed == 0l"}) - public SimpleTimer start() { + public synchronized SimpleTimer start() { elapsed = 0l; restart(); return this; @@ -73,7 +73,7 @@ public class SimpleTimer { */ @Requires("running == false") @Ensures("result != null") - public SimpleTimer restart() { + public synchronized SimpleTimer restart() { running = true; startTime = currentTime(); return this; @@ -101,7 +101,7 @@ public class SimpleTimer { */ @Requires("running == true") @Ensures({"result != null", "elapsed >= old(elapsed)", "running == false"}) - public SimpleTimer stop() { + public synchronized SimpleTimer stop() { running = false; elapsed += currentTime() - startTime; return this; @@ -116,11 +116,10 @@ public class SimpleTimer { @Ensures({ "result >= (elapsed/1000.0)", "result >= 0"}) - public double getElapsedTime() { + public synchronized double getElapsedTime() { return (running ? (currentTime() - startTime + elapsed) : elapsed) / 1000.0; } - public void printElapsedTime(PrintStream out) { out.printf("SimpleTimer %s: %.2f%n", name, getElapsedTime()); } diff --git a/public/java/src/org/broadinstitute/sting/utils/sam/AlignmentUtils.java b/public/java/src/org/broadinstitute/sting/utils/sam/AlignmentUtils.java index 344eccb83..b8e892101 100644 --- a/public/java/src/org/broadinstitute/sting/utils/sam/AlignmentUtils.java +++ b/public/java/src/org/broadinstitute/sting/utils/sam/AlignmentUtils.java @@ -353,7 +353,7 @@ public class AlignmentUtils { break; case D: case N: - refPos++; + refPos += elementLength; break; case M: for ( int jjj = 0; jjj < elementLength; jjj++ ) { From 2ebdff074c6240438daf79ba21b3b2e8c5467afa Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Thu, 13 Oct 2011 18:01:51 -0400 Subject: [PATCH 02/20] Update MD5s for SOLiD recalibration -- MD5 db had spelling error; fixed -- Bug in AlignmentUtils resulted in some bases not being color space corrected. The integration test caught the change, and it's clear that the new version is correct, as the prev. version was not considering the last the N qualities for reads with a ND operation. --- public/java/test/org/broadinstitute/sting/MD5DB.java | 2 +- .../recalibration/RecalibrationWalkersIntegrationTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/public/java/test/org/broadinstitute/sting/MD5DB.java b/public/java/test/org/broadinstitute/sting/MD5DB.java index 0194e114a..2fd8f8b6d 100644 --- a/public/java/test/org/broadinstitute/sting/MD5DB.java +++ b/public/java/test/org/broadinstitute/sting/MD5DB.java @@ -224,7 +224,7 @@ public class MD5DB { if ( ! expectedMD5.equals(filemd5sum) ) { // we are going to fail for real in assertEquals (so we are counted by the testing framework). // prepare ourselves for the comparison - System.out.printf("##### Test %s is going fail #####%n", name); + System.out.printf("##### Test %s is going to fail #####%n", name); String pathToExpectedMD5File = getMD5FilePath(expectedMD5, "[No DB file found]"); String pathToFileMD5File = getMD5FilePath(filemd5sum, "[No DB file found]"); System.out.printf("##### Path to expected file (MD5=%s): %s%n", expectedMD5, pathToExpectedMD5File); diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/recalibration/RecalibrationWalkersIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/recalibration/RecalibrationWalkersIntegrationTest.java index b2833b935..cbcd5835f 100755 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/recalibration/RecalibrationWalkersIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/recalibration/RecalibrationWalkersIntegrationTest.java @@ -89,7 +89,7 @@ public class RecalibrationWalkersIntegrationTest extends WalkerTest { @DataProvider(name = "trtestdata") public Object[][] createTRTestData() { new TRTest( validationDataLocation + "NA12892.SLX.SRP000031.2009_06.selected.bam", "2864f231fab7030377f3c8826796e48f" ); - new TRTest( validationDataLocation + "NA19240.chr1.BFAST.SOLID.bam", "c164dd635721ba6df3f06dac1877c32d"); + new TRTest( validationDataLocation + "NA19240.chr1.BFAST.SOLID.bam", "d04cf1f6df486e45226ebfbf93a188a5"); new TRTest( validationDataLocation + "NA12873.454.SRP000031.2009_06.chr1.10_20mb.bam", "74314e5562c1a65547bb0edaacffe602" ); new TRTest( validationDataLocation + "NA12878.1kg.p2.chr1_10mb_11_mb.allTechs.bam", "2a37c6001826bfabf87063b1dfcf594f" ); return TRTest.getTests(TRTest.class); @@ -200,7 +200,7 @@ public class RecalibrationWalkersIntegrationTest extends WalkerTest { @Test public void testTableRecalibratorSolidIndelsRemoveRefBias() { HashMap e = new HashMap(); - e.put( validationDataLocation + "NA19240.chr1.BFAST.SOLID.bam", "7d5edb75b176e4151de225f699719ee4" ); + e.put( validationDataLocation + "NA19240.chr1.BFAST.SOLID.bam", "2ad4c17ac3ed380071137e4e53a398a5" ); for ( Map.Entry entry : e.entrySet() ) { String bam = entry.getKey(); From edfd6f8a06bfbaaddfbb26677dc98f33af9f6318 Mon Sep 17 00:00:00 2001 From: David Roazen Date: Thu, 13 Oct 2011 21:32:52 -0400 Subject: [PATCH 03/20] Removing a public -> private dependency from the test suite. The public integration test VariantContextIntegrationTest was dependent on the private walker TestVariantContextWalker. Moved this walker to public/java/test (NOT public/java/src, since this walker is only used by the test suite) to avoid errors during public-only tests. --- .../walkers/qc/TestVariantContextWalker.java | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100755 public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java new file mode 100755 index 000000000..7607049db --- /dev/null +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/qc/TestVariantContextWalker.java @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2010 The Broad Institute + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR + * THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +package org.broadinstitute.sting.gatk.walkers.qc; + +import org.broadinstitute.sting.commandline.Argument; +import org.broadinstitute.sting.commandline.ArgumentCollection; +import org.broadinstitute.sting.commandline.Output; +import org.broadinstitute.sting.gatk.arguments.DbsnpArgumentCollection; +import org.broadinstitute.sting.gatk.arguments.StandardVariantContextInputArgumentCollection; +import org.broadinstitute.sting.gatk.contexts.AlignmentContext; +import org.broadinstitute.sting.gatk.contexts.ReferenceContext; +import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker; +import org.broadinstitute.sting.gatk.refdata.VariantContextAdaptors; +import org.broadinstitute.sting.gatk.walkers.Reference; +import org.broadinstitute.sting.gatk.walkers.RodWalker; +import org.broadinstitute.sting.gatk.walkers.Window; +import org.broadinstitute.sting.utils.codecs.vcf.VCFWriter; +import org.broadinstitute.sting.utils.variantcontext.VariantContext; + +import java.io.PrintStream; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.List; + +/** + * Test routine for new VariantContext object + */ +@Reference(window=@Window(start=-20,stop=1)) +public class TestVariantContextWalker extends RodWalker { + @Output + PrintStream out; + + @ArgumentCollection + protected StandardVariantContextInputArgumentCollection variantCollection = new StandardVariantContextInputArgumentCollection(); + + @Argument(fullName="takeFirstOnly", doc="Only take the first second at a locus, as opposed to all", required=false) + boolean takeFirstOnly = false; + + @Argument(fullName="onlyContextsOfType", doc="Only take variant contexts of this type", required=false) + VariantContext.Type onlyOfThisType = null; + + @Argument(fullName="onlyContextsStartinAtCurrentPosition", doc="Only take variant contexts at actually start at the current position, excluding those at span to the current location but start earlier", required=false) + boolean onlyContextsStartinAtCurrentPosition = false; + + @Argument(fullName="printPerLocus", doc="If true, we'll print the variant contexts, in addition to counts", required=false) + boolean printContexts = false; + + @Argument(fullName="outputVCF", doc="If provided, we'll convert the first input context into a VCF", required=false) + VCFWriter writer = null; + + private boolean wroteHeader = false; + + public Integer map(RefMetaDataTracker tracker, ReferenceContext ref, AlignmentContext context) { + if ( ref == null ) + return 0; + else { + EnumSet allowedTypes = onlyOfThisType == null ? null : EnumSet.of(onlyOfThisType); + + int n = 0; + List contexts; + if ( onlyContextsStartinAtCurrentPosition ) + contexts = tracker.getValues(variantCollection.variants, context.getLocation()); + else // ! onlyContextsStartinAtCurrentPosition + contexts = tracker.getValues(variantCollection.variants); + + for ( VariantContext vc : contexts ) { + if ( allowedTypes == null || allowedTypes.contains(vc.getType()) ) { + // we need to trigger decoding of the genotype string to pass integration tests + vc.getGenotypes(); + + if ( writer != null && n == 0 ) { + if ( ! wroteHeader ) { + writer.writeHeader(VariantContextAdaptors.createVCFHeader(null, vc)); + wroteHeader = true; + } + + writer.add(vc); + } + + n++; + if ( printContexts ) out.printf(" %s%n", vc); + if ( takeFirstOnly ) break; + } + } + + if ( n > 0 && printContexts ) { + out.printf("%s => had %d variant context objects%n", context.getLocation(), n); + out.printf("---------------------------------------------%n"); + } + + return n; + } + } + + public Integer reduceInit() { + return 0; + } + + public Integer reduce(Integer point, Integer sum) { + return point + sum; + } + + @Override + public void onTraversalDone(Integer result) { + // Double check traversal result to make count is the same. + // TODO: Is this check necessary? + out.println("[REDUCE RESULT] Traversal result is: " + result); + } +} From 442d33ba186d0c008a83b0da3b20dec0c91f070a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Wed, 28 Sep 2011 16:23:54 -0400 Subject: [PATCH 04/20] Enable testing of the jars produced by the packaging system. -Added targets to run unit and integration tests on the fully-packaged GATK jar, and pipeline tests on the fully-packaged Queue jar. Once enabled in Bamboo, these will provide greatly-enhanced protection against breakage in the binary release. -Unconditionally include all of the subset of org.broadinstitute.sting included in the intermediate jars GenomeAnalysisTK.jar, StingUtils.jar, etc. in the final, fully-packaged jar. This: * is necessary to get tests to run on the fully-packaged jar * decreases the chances of a class that is a runtime-only dependency getting left out of the binary release * only slightly increases the size of the binary release (before: 9352465 bytes, after: 10985482 bytes) --- build.xml | 156 ++++++++++++++++++++++++--------- public/packages/GATKEngine.xml | 10 +-- 2 files changed, 116 insertions(+), 50 deletions(-) diff --git a/build.xml b/build.xml index a4caa96c9..16bc8d9b4 100644 --- a/build.xml +++ b/build.xml @@ -67,6 +67,8 @@ + + @@ -118,6 +120,16 @@ + + + + + + + + + + @@ -827,10 +839,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -851,6 +907,7 @@ - - - - - - - - - - @@ -902,6 +949,10 @@ + + + + @@ -914,62 +965,81 @@ + + + + + + + + + + + + + + + + + + + - + - + - + - + - + - + - + - + - + - + - - + + - - + + - - + + - - + + @@ -1027,14 +1097,14 @@ - + - - + + @@ -1043,8 +1113,8 @@ - - + + @@ -1055,11 +1125,11 @@ - + - + @@ -1068,7 +1138,7 @@ - + @@ -1076,14 +1146,14 @@ - - - + + + - - - + + + @@ -1119,7 +1189,7 @@ - + diff --git a/public/packages/GATKEngine.xml b/public/packages/GATKEngine.xml index 4364988e7..e6056408e 100644 --- a/public/packages/GATKEngine.xml +++ b/public/packages/GATKEngine.xml @@ -26,14 +26,10 @@ - - - - - - + + - + From bd8bb93811aa3ec20d6777dce84c6a62b8d35f3d Mon Sep 17 00:00:00 2001 From: David Roazen Date: Fri, 14 Oct 2011 20:04:42 -0400 Subject: [PATCH 05/20] Split RScriptExecutorUnitTest into public and private test classes. We can't have a public test that depends on both public and private code/data -- the new release system needs to do public-only tests, and will catch this sort of thing. --- .../utils/R/RScriptExecutorUnitTest.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java b/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java index 1bbf74db9..836a4473f 100644 --- a/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/utils/R/RScriptExecutorUnitTest.java @@ -26,21 +26,12 @@ package org.broadinstitute.sting.utils.R; import org.apache.commons.io.FileUtils; import org.broadinstitute.sting.BaseTest; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffElement; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffEngine; -import org.broadinstitute.sting.gatk.walkers.diffengine.DiffNode; -import org.broadinstitute.sting.gatk.walkers.diffengine.Difference; import org.broadinstitute.sting.utils.exceptions.UserException; -import org.testng.Assert; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; /** @@ -49,7 +40,6 @@ import java.util.List; public class RScriptExecutorUnitTest extends BaseTest { final static String testrscript = "print(\"hello, world\")\n"; final static String publicRScript = "plot_Tranches.R"; - final static String privateRScript = "variantCallQC.R"; // -------------------------------------------------------------------------------- // @@ -74,17 +64,6 @@ public class RScriptExecutorUnitTest extends BaseTest { @Test public void testPublic() { testOne(publicRScript, null, null, true); } - @Test - public void testPrivate() { testOne(privateRScript, null, null, true); } - - // make sure we don't break finding something in private by adding another directory - @Test - public void testPrivateWithAdditionalPath1() { testOne(privateRScript, null, "dist", true); } - - // make sure we don't break finding something in private by adding another directory - @Test - public void testPrivateWithAdditionalPath2() { testOne(privateRScript, null, "doesNotExist", true); } - @Test(expectedExceptions = UserException.class) public void testNonExistantScriptException() { testOne("does_not_exist.R", null, null, true); } From c756da179888dbca8d157330f14f7737d596f89a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Fri, 14 Oct 2011 22:59:12 -0400 Subject: [PATCH 06/20] Remove unnecessary debugger port listening from GATKDoclet invocation. This caused a race condition between concurrently-running bamboo plans. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index 16bc8d9b4..babf31506 100644 --- a/build.xml +++ b/build.xml @@ -519,7 +519,7 @@ docletpathref="doclet.classpath" classpathref="external.dependencies" classpath="${java.classes}" - additionalparam="${gatkdocs.include.hidden.arg} -private -build-timestamp "${build.timestamp}" -absolute-version ${build.version} -quiet -J-Xdebug -J-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"> + additionalparam="${gatkdocs.include.hidden.arg} -private -build-timestamp "${build.timestamp}" -absolute-version ${build.version} -quiet"> From 46badee76a16073b286d3809f900857e0fc3fb7a Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sat, 15 Oct 2011 20:20:41 -0400 Subject: [PATCH 07/20] Intentionally breaking the binary release as a test. This is to test the ability of the new release system to detect the kinds of packaging errors we couldn't detect before (in this case, missing codecs). If all goes as expected, "GSA-Stable: Release" will fail, preventing the binary release and github from getting updated, while "GSA-Stable: All Tests" and all of the other old test plans will pass. Will revert this in a bit -- if the system works as it should, our users will never see it until after it's been reverted. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index babf31506..75eaa3ffb 100644 --- a/build.xml +++ b/build.xml @@ -541,7 +541,7 @@ - + From dcd4eee15f7eb350a25f6e2c1fabd89b6e95b47c Mon Sep 17 00:00:00 2001 From: David Roazen Date: Sun, 16 Oct 2011 07:24:33 -0400 Subject: [PATCH 08/20] Revert "Intentionally breaking the binary release as a test." The test was successful -- the packaging error was detected and prevented from propagating out into the binary/source releases. This reverts commit 8f2a1462d94ce50fb7c1b1d0c40142b109e9c38e. --- build.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.xml b/build.xml index 75eaa3ffb..babf31506 100644 --- a/build.xml +++ b/build.xml @@ -541,7 +541,7 @@ - + From fd4540cd323e85396a4ba5d2bb2b8ee52f88e766 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 17 Oct 2011 13:37:55 -0400 Subject: [PATCH 09/20] Fixed extraordinarily subtle race condition with contracts invariant -- all of the methods in the class must be synchronized or the internal state can be inconsistent with the contract invariant when entering the class in a non-synchronized method, even when that method doesn't care about the object's internal state --- .../org/broadinstitute/sting/utils/SimpleTimer.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java b/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java index 6d228640f..a5ac10250 100644 --- a/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java +++ b/public/java/src/org/broadinstitute/sting/utils/SimpleTimer.java @@ -46,7 +46,7 @@ public class SimpleTimer { * @return the name associated with this timer */ @Ensures("result != null") - public String getName() { + public synchronized String getName() { return name; } @@ -82,14 +82,14 @@ public class SimpleTimer { /** * @return is this timer running? */ - public boolean isRunning() { + public synchronized boolean isRunning() { return running; } /** * @return A convenience function to obtain the current time in milliseconds from this timer */ - public long currentTime() { + public synchronized long currentTime() { return System.currentTimeMillis(); } @@ -119,8 +119,4 @@ public class SimpleTimer { public synchronized double getElapsedTime() { return (running ? (currentTime() - startTime + elapsed) : elapsed) / 1000.0; } - - public void printElapsedTime(PrintStream out) { - out.printf("SimpleTimer %s: %.2f%n", name, getElapsedTime()); - } } From 4108a294f76e051f9c14cc8884e49a90a46411c9 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 17 Oct 2011 13:58:46 -0400 Subject: [PATCH 10/20] Better error message when a RodBinding file doesn't exist --- .../sting/commandline/ArgumentTypeDescriptor.java | 2 +- .../broadinstitute/sting/utils/exceptions/UserException.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index 5fff8f609..94157963f 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -380,7 +380,7 @@ class RodBindingArgumentTypeDescriptor extends ArgumentTypeDescriptor { if ( tribbleType == null ) if ( ! file.canRead() | ! file.isFile() ) { - throw new UserException.BadArgumentValue(name, "Couldn't read file to determine type: " + file); + throw new UserException.CouldNotReadInputFile(file, "file does exist or couldn't be read"); } else { throw new UserException.CommandLineException( String.format("No tribble type was provided on the command line and the type of the file could not be determined dynamically. " + diff --git a/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java b/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java index 274c64f42..9d131ae0c 100755 --- a/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java +++ b/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java @@ -111,6 +111,10 @@ public class UserException extends ReviewedStingException { super(String.format("Couldn't read file because %s caused by %s", message, e.getMessage())); } + public CouldNotReadInputFile(File file) { + super(String.format("Couldn't read file %s", file.getAbsolutePath())); + } + public CouldNotReadInputFile(File file, String message) { super(String.format("Couldn't read file %s because %s", file.getAbsolutePath(), message)); } From 6b02354d8467d8435d4077195ac0b10a4a1ef3b7 Mon Sep 17 00:00:00 2001 From: Ryan Poplin Date: Mon, 17 Oct 2011 14:34:52 -0400 Subject: [PATCH 11/20] Adding a new getter in VariantsToTable to extract the indel event length. --- .../gatk/walkers/variantutils/VariantsToTable.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java index 81d0c36ac..7b0b81d2a 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java @@ -27,6 +27,7 @@ package org.broadinstitute.sting.gatk.walkers.variantutils; import org.broadinstitute.sting.commandline.*; import org.broadinstitute.sting.gatk.arguments.StandardVariantContextInputArgumentCollection; import org.broadinstitute.sting.utils.MathUtils; +import org.broadinstitute.sting.utils.variantcontext.Allele; import org.broadinstitute.sting.utils.variantcontext.VariantContext; import org.broadinstitute.sting.gatk.contexts.AlignmentContext; import org.broadinstitute.sting.gatk.contexts.ReferenceContext; @@ -294,6 +295,14 @@ public class VariantsToTable extends RodWalker { return x.toString(); } }); + getters.put("EVENTLENGTH", new Getter() { public String get(VariantContext vc) { + int maxLength = 0; + for ( final Allele a : vc.getAlternateAlleles() ) { + final int length = a.length() - vc.getReference().length(); + if( Math.abs(length) > Math.abs(maxLength) ) { maxLength = length; } + } + return Integer.toString(maxLength); + }}); getters.put("QUAL", new Getter() { public String get(VariantContext vc) { return Double.toString(vc.getPhredScaledQual()); } }); getters.put("TRANSITION", new Getter() { public String get(VariantContext vc) { if ( vc.isSNP() && vc.isBiallelic() ) @@ -304,7 +313,6 @@ public class VariantsToTable extends RodWalker { getters.put("FILTER", new Getter() { public String get(VariantContext vc) { return vc.isNotFiltered() ? "PASS" : Utils.join(",", vc.getFilters()); } }); - getters.put("HET", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHetCount()); } }); getters.put("HOM-REF", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHomRefCount()); } }); getters.put("HOM-VAR", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHomVarCount()); } }); From a7cf9cdc67e14a9298e013df42beda06caba7595 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 17 Oct 2011 15:25:35 -0400 Subject: [PATCH 12/20] Fixing error message typo --- .../sting/commandline/ArgumentTypeDescriptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index 94157963f..14275c57b 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -380,7 +380,7 @@ class RodBindingArgumentTypeDescriptor extends ArgumentTypeDescriptor { if ( tribbleType == null ) if ( ! file.canRead() | ! file.isFile() ) { - throw new UserException.CouldNotReadInputFile(file, "file does exist or couldn't be read"); + throw new UserException.CouldNotReadInputFile(file, "file does't exist or couldn't be read"); } else { throw new UserException.CommandLineException( String.format("No tribble type was provided on the command line and the type of the file could not be determined dynamically. " + From ec911ce5bb8c3873dbfd0a0eb409aaed5d6a82a6 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 17 Oct 2011 15:27:22 -0400 Subject: [PATCH 13/20] Even better error messages --- .../sting/commandline/ArgumentTypeDescriptor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index 14275c57b..b54e7c7b1 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -379,8 +379,10 @@ class RodBindingArgumentTypeDescriptor extends ArgumentTypeDescriptor { } if ( tribbleType == null ) - if ( ! file.canRead() | ! file.isFile() ) { - throw new UserException.CouldNotReadInputFile(file, "file does't exist or couldn't be read"); + if ( ! file.exists() ) { + throw new UserException.CouldNotReadInputFile(file, "file does not exist"); + } else if ( ! file.canRead() | ! file.isFile() ) { + throw new UserException.CouldNotReadInputFile(file, "file could not be read"); } else { throw new UserException.CommandLineException( String.format("No tribble type was provided on the command line and the type of the file could not be determined dynamically. " + From c1329c4ddee4ce4330ff5046003d9e389f0c6f43 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 17 Oct 2011 15:29:45 -0400 Subject: [PATCH 14/20] Fixing a binary to logical or --- .../sting/commandline/ArgumentTypeDescriptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index b54e7c7b1..d1d9cf7fe 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/public/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -381,7 +381,7 @@ class RodBindingArgumentTypeDescriptor extends ArgumentTypeDescriptor { if ( tribbleType == null ) if ( ! file.exists() ) { throw new UserException.CouldNotReadInputFile(file, "file does not exist"); - } else if ( ! file.canRead() | ! file.isFile() ) { + } else if ( ! file.canRead() || ! file.isFile() ) { throw new UserException.CouldNotReadInputFile(file, "file could not be read"); } else { throw new UserException.CommandLineException( From 1e6794c53985f02b27d0f7418c0d1784b28ad754 Mon Sep 17 00:00:00 2001 From: Ryan Poplin Date: Mon, 17 Oct 2011 15:56:02 -0400 Subject: [PATCH 16/20] fixing typo in VariantsToTable docs --- .../sting/gatk/walkers/variantutils/VariantsToTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java index 7b0b81d2a..bf16b2dfb 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java @@ -134,7 +134,7 @@ public class VariantsToTable extends RodWalker { /** * By default, this tool throws a UserException when it encounters a field without a value in some record. This - * is generally useful when you mistype -F CHROM, so that you get a friendly warning about CHRMO not being + * is generally useful when you mistype -F CHROM, so that you get a friendly warning about CHROM not being * found before the tool runs through 40M 1000G records. However, in some cases you genuinely want to allow such * fields (e.g., AC not being calculated for filtered records, if included). When provided, this argument * will cause VariantsToTable to write out NA values for missing fields instead of throwing an error. From 1a92ee3593417941a31b32e60cb34796d9bd5d95 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Tue, 18 Oct 2011 10:57:02 -0400 Subject: [PATCH 18/20] No longer adds a binding of ID -> . when the ID field is dot in the VCF -- Really we should make ID a primary key in VariantContext. Putting it into the attributes is just annoying now --- .../sting/utils/codecs/vcf/AbstractVCFCodec.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/AbstractVCFCodec.java b/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/AbstractVCFCodec.java index c86b91b79..6b101ca74 100755 --- a/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/AbstractVCFCodec.java +++ b/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/AbstractVCFCodec.java @@ -381,7 +381,8 @@ public abstract class AbstractVCFCodec implements FeatureCodec, NameAwareCodec, } } - attributes.put(VariantContext.ID_KEY, id); + if ( ! id.equals(VCFConstants.EMPTY_ID_FIELD) ) + attributes.put(VariantContext.ID_KEY, id); return attributes; } From f77f2eeb7d1abf58766f430974f0c7c395740f0e Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Tue, 18 Oct 2011 13:04:43 -0400 Subject: [PATCH 19/20] Fix for new ID structure --- .../sting/gatk/walkers/variantutils/VariantsToTable.java | 1 + 1 file changed, 1 insertion(+) diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java index bf16b2dfb..5dbd3f5fd 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/VariantsToTable.java @@ -313,6 +313,7 @@ public class VariantsToTable extends RodWalker { getters.put("FILTER", new Getter() { public String get(VariantContext vc) { return vc.isNotFiltered() ? "PASS" : Utils.join(",", vc.getFilters()); } }); + getters.put("ID", new Getter() { public String get(VariantContext vc) { return vc.hasID() ? vc.getID() : "."; } }); getters.put("HET", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHetCount()); } }); getters.put("HOM-REF", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHomRefCount()); } }); getters.put("HOM-VAR", new Getter() { public String get(VariantContext vc) { return Integer.toString(vc.getHomVarCount()); } }); From e5fc82854609f90df8be3704e6f3cd5f482b5a91 Mon Sep 17 00:00:00 2001 From: Menachem Fromer Date: Tue, 18 Oct 2011 14:47:39 -0400 Subject: [PATCH 20/20] With Khalid's implicit approval, I have removed this line that overrides the memory limit of the VCF-gathering function, so that the inherited limit remains --- .../sting/queue/extensions/gatk/VcfGatherFunction.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/VcfGatherFunction.scala b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/VcfGatherFunction.scala index d70022147..70046c913 100644 --- a/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/VcfGatherFunction.scala +++ b/public/scala/src/org/broadinstitute/sting/queue/extensions/gatk/VcfGatherFunction.scala @@ -36,8 +36,6 @@ class VcfGatherFunction extends CombineVariants with GatherFunction { private lazy val originalGATK = this.originalFunction.asInstanceOf[CommandLineGATK] override def freezeFieldValues { - this.memoryLimit = Some(1) - this.jarFile = this.originalGATK.jarFile this.reference_sequence = this.originalGATK.reference_sequence this.intervals = this.originalGATK.intervals