Merged bug fix from Stable into Unstable

protected/java/test/org/broadinstitute/sting/gatk/walkers/bqsr/BQSRIntegrationTest.java

@@ -28,6 +28,17 @@ public class BQSRIntegrationTest extends WalkerTest {
             this.md5 = md5;
         }
 
+        public String getCommandLine() {
+            return " -T BaseRecalibrator" +
+                    " -R " + reference +
+                    " -I " + bam +
+                    " -L " + interval +
+                    args +
+                    " --no_plots" +
+                    " -knownSites " + (reference.equals(b36KGReference) ? b36dbSNP129 : hg18dbSNP132) +
+                    " -o %s";
+        }
+
         @Override
         public String toString() {
             return String.format("BQSR(bam='%s', args='%s')", bam, args);
@@ -59,16 +70,14 @@ public class BQSRIntegrationTest extends WalkerTest {
     @Test(dataProvider = "BQSRTest")
     public void testBQSR(BQSRTest params) {
         WalkerTestSpec spec = new WalkerTestSpec(
-                " -T BaseRecalibrator" +
+                params.getCommandLine(),
-                        " -R " + params.reference +
-                        " -I " + params.bam +
-                        " -L " + params.interval +
-                        params.args +
-                        " --no_plots" +
-                        " -knownSites " + (params.reference.equals(b36KGReference) ? b36dbSNP129 : hg18dbSNP132) +
-                        " -o %s",
                 Arrays.asList(params.md5));
         executeTest("testBQSR-"+params.args, spec).getFirst();
+
+        WalkerTestSpec specNT2 = new WalkerTestSpec(
+                params.getCommandLine() + " -nt 2",
+                Arrays.asList(params.md5));
+        executeTest("testBQSR-nt2-"+params.args, specNT2).getFirst();
     }
 
     @Test

public/java/src/org/broadinstitute/sting/gatk/GenomeAnalysisEngine.java

@@ -281,7 +281,14 @@ public class GenomeAnalysisEngine {
      * @return An instance of the walker.
      */
     public Walker<?, ?> getWalkerByName(String walkerName) {
-        return walkerManager.createByName(walkerName);
+        try {
+            return walkerManager.createByName(walkerName);
+        } catch ( UserException e ) {
+            if ( isGATKLite() && GATKLiteUtils.isAvailableOnlyInFullGATK(walkerName) ) {
+                e = new UserException.NotSupportedInGATKLite("the " + walkerName + " walker is available only in the full version of the GATK");
+            }
+            throw e;
+        }
     }
 
     /**

public/java/src/org/broadinstitute/sting/gatk/executive/HierarchicalMicroScheduler.java

@@ -17,7 +17,10 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.Queue;
-import java.util.concurrent.*;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.FutureTask;
 
 /**
  * A microscheduler that schedules shards according to a tree-like structure.
@@ -40,6 +43,11 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
 
     private final Queue<TreeReduceTask> reduceTasks = new LinkedList<TreeReduceTask>();
 
+    /**
+     * An exception that's occurred in this traversal.  If null, no exception has occurred.
+     */
+    private RuntimeException error = null;
+
     /**
      * Queue of incoming shards.
      */
@@ -90,13 +98,11 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
         ReduceTree reduceTree = new ReduceTree(this);
         initializeWalker(walker);
 
-        //
-        // exception handling here is a bit complex.  We used to catch and rethrow exceptions all over
-        // the place, but that just didn't work well.  Now we have a specific execution exception (inner class)
-        // to use for multi-threading specific exceptions.  All RuntimeExceptions that occur within the threads are rethrown
-        // up the stack as their underlying causes
-        //
         while (isShardTraversePending() || isTreeReducePending()) {
+            // Check for errors during execution.
+            if(hasTraversalErrorOccurred())
+                throw getTraversalError();
+
             // Too many files sitting around taking up space?  Merge them.
             if (isMergeLimitExceeded())
                 mergeExistingOutput(false);
@@ -123,8 +129,12 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
             result = reduceTree.getResult().get();
             notifyTraversalDone(walker,result);
         }
-        catch( InterruptedException ex ) { handleException(ex); }
+        catch (ReviewedStingException ex) {
-        catch( ExecutionException ex ) { handleException(ex); }
+            throw ex;
+        }
+        catch (Exception ex) {
+            throw new ReviewedStingException("Unable to retrieve result", ex);
+        }
 
         // do final cleanup operations
         outputTracker.close();
@@ -255,8 +265,7 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
                     // Specifically catch Tribble I/O exceptions and rethrow them as Reviewed.  We don't expect
                     // any issues here because we created the Tribble output file mere moments ago and expect it to
                     // be completely valid.
-                    final String reason = ex.getMessage();
+                    throw new ReviewedStingException("Unable to merge temporary Tribble output file.",ex);
-                    throw new ReviewedStingException("Unable to merge temporary Tribble output file" + (reason == null ? "." : (" (" + reason + ").")), ex);
                 }
             }
         }
@@ -328,39 +337,30 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
     }
 
     /**
-     * Handle an exception that occurred in a worker thread as needed by this scheduler.
+     * Detects whether an execution error has occurred.
-     *
+     * @return True if an error has occurred.  False otherwise.
-     * The way to use this function in a worker is:
-     *
-     * try { doSomeWork();
-     * catch ( InterruptedException ex ) { hms.handleException(ex); }
-     * catch ( ExecutionException ex ) { hms.handleException(ex); }
-     *
-     * @param ex the exception that occurred in the worker thread
      */
-    protected final void handleException(InterruptedException ex) {
+    private synchronized boolean hasTraversalErrorOccurred() {
-        throw new HierarchicalMicroScheduler.ExecutionFailure("Hierarchical reduce interrupted", ex);
+        return error != null;
+    }
+
+    private synchronized RuntimeException getTraversalError() {
+        if(!hasTraversalErrorOccurred())
+            throw new ReviewedStingException("User has attempted to retrieve a traversal error when none exists");
+        return error;
     }
 
     /**
-     * Handle an exception that occurred in a worker thread as needed by this scheduler.
+     * Allows other threads to notify of an error during traversal.
-     *
-     * The way to use this function in a worker is:
-     *
-     * try { doSomeWork();
-     * catch ( InterruptedException ex ) { hms.handleException(ex); }
-     * catch ( ExecutionException ex ) { hms.handleException(ex); }
-     *
-     * @param ex the exception that occurred in the worker thread
      */
-    protected final void handleException(ExecutionException ex) {
+    protected synchronized void notifyOfTraversalError(Throwable error) {
-        if ( ex.getCause() instanceof RuntimeException )
+        // If the error is already a Runtime, pass it along as is.  Otherwise, wrap it.
-            // if the cause was a runtime exception that's what we want to send up the stack
+        if (error instanceof RuntimeException)
-            throw (RuntimeException )ex.getCause();
+            this.error = (RuntimeException)error;
         else
-            throw new HierarchicalMicroScheduler.ExecutionFailure("Hierarchical reduce failed", ex);
+            this.error = new ReviewedStingException("An error occurred during the traversal.", error);
-    }
 
+    }
 
 
     /** A small wrapper class that provides the TreeReducer interface along with the FutureTask semantics. */
@@ -381,17 +381,6 @@ public class HierarchicalMicroScheduler extends MicroScheduler implements Hierar
         }
     }
 
-    /**
-     * A specific exception class for HMS-specific failures such as
-     * Interrupted or ExecutionFailures that aren't clearly the fault
-     * of the underlying walker code
-     */
-    public static class ExecutionFailure extends ReviewedStingException {
-        public ExecutionFailure(final String s, final Throwable throwable) {
-            super(s, throwable);
-        }
-    }
-
     /**
      * Used by the ShardTraverser to report time consumed traversing a given shard.
      *

public/java/src/org/broadinstitute/sting/gatk/executive/ShardTraverser.java

@@ -80,8 +80,8 @@ public class ShardTraverser implements Callable {
 
             return accumulator;
         } catch(Throwable t) {
-            // Notify that an exception has occurred
+            // Notify that an exception has occurred and rethrow it.
-            microScheduler.handleException(new ExecutionException(t));
+            microScheduler.notifyOfTraversalError(t);
             throw new RuntimeException(t);
         } finally {
             synchronized(this) {

public/java/src/org/broadinstitute/sting/gatk/executive/TreeReducer.java

@@ -79,8 +79,14 @@ public class TreeReducer implements Callable {
             else
                 result = walker.treeReduce( lhs.get(), rhs.get() );
         }
-        catch( InterruptedException ex ) { microScheduler.handleException(ex); }
+        catch( InterruptedException ex ) {
-        catch( ExecutionException ex ) { microScheduler.handleException(ex); }
+            microScheduler.notifyOfTraversalError(ex);
+            throw new ReviewedStingException("Hierarchical reduce interrupted", ex);
+        }
+        catch( ExecutionException ex ) {
+            microScheduler.notifyOfTraversalError(ex);
+            throw new ReviewedStingException("Hierarchical reduce failed", ex);
+        }
 
         final long endTime = System.currentTimeMillis();
 

public/java/src/org/broadinstitute/sting/gatk/walkers/bqsr/BaseRecalibrator.java

@@ -58,13 +58,11 @@ import java.util.ArrayList;
  * of poor base quality. This walker generates tables based on various user-specified covariates (such as read group,
  * reported quality score, cycle, and dinucleotide). Since there is a large amount of data one can then calculate an empirical
  * probability of error given the particular covariates seen at this site, where p(error) = num mismatches / num observations.
- * The output file is a CSV list of (the several covariate values, num observations, num mismatches, empirical quality score).
+ * The output file is a table (of the several covariate values, num observations, num mismatches, empirical quality score).
  * <p>
  * Note: ReadGroupCovariate and QualityScoreCovariate are required covariates and will be added for the user regardless of whether or not they were specified.
  *
  * <p>
- * See the GATK wiki for a tutorial and example recalibration accuracy plots.
- * http://www.broadinstitute.org/gsa/wiki/index.php/Base_quality_score_recalibration
  *
  * <h2>Input</h2>
  * <p>
@@ -120,9 +118,9 @@ public class BaseRecalibrator extends LocusWalker<Long, Long> implements TreeRed
 
     private int minimumQToUse;
 
-    protected static final String SKIP_RECORD_ATTRIBUTE = "SKIP";                                                         // used to label reads that should be skipped.
+    protected static final String SKIP_RECORD_ATTRIBUTE = "SKIP";                                                       // used to label reads that should be skipped.
-    protected static final String SEEN_ATTRIBUTE = "SEEN";                                                                // used to label reads as processed.
+    protected static final String SEEN_ATTRIBUTE = "SEEN";                                                              // used to label reads as processed.
-    protected static final String COVARS_ATTRIBUTE = "COVARS";                                                            // used to store covariates array as a temporary attribute inside GATKSAMRecord.\
+    protected static final String COVARS_ATTRIBUTE = "COVARS";                                                          // used to store covariates array as a temporary attribute inside GATKSAMRecord.\
 
     private static final String NO_DBSNP_EXCEPTION = "This calculation is critically dependent on being able to skip over known variant sites. Please provide a VCF file containing known sites of genetic variation.";
 

public/java/src/org/broadinstitute/sting/gatk/walkers/bqsr/ContextCovariate.java

@@ -51,10 +51,6 @@ public class ContextCovariate implements StandardCovariate {
     private static final int LENGTH_BITS = 4;
     private static final int LENGTH_MASK = 15;
 
-    // temporary lists to use for creating context covariate keys
-    private final ArrayList<Integer> mismatchKeys = new ArrayList<Integer>(200);
-    private final ArrayList<Integer> indelKeys = new ArrayList<Integer>(200);
-
     // the maximum context size (number of bases) permitted; we need to keep the leftmost base free so that values are
     // not negative and we reserve 4 more bits to represent the length of the context; it takes 2 bits to encode one base.
     static final private int MAX_DNA_CONTEXT = 13;
@@ -91,10 +87,8 @@ public class ContextCovariate implements StandardCovariate {
         if (negativeStrand)
             bases = BaseUtils.simpleReverseComplement(bases);
 
-        mismatchKeys.clear();
+        final ArrayList<Integer> mismatchKeys = contextWith(bases, mismatchesContextSize, mismatchesKeyMask);
-        indelKeys.clear();
+        final ArrayList<Integer> indelKeys = contextWith(bases, indelsContextSize, indelsKeyMask);
-        contextWith(bases, mismatchesContextSize, mismatchKeys, mismatchesKeyMask);
-        contextWith(bases, indelsContextSize, indelKeys, indelsKeyMask);
 
         final int readLength = bases.length;
         for (int i = 0; i < readLength; i++) {
@@ -139,17 +133,19 @@ public class ContextCovariate implements StandardCovariate {
      *
      * @param bases       the bases in the read to build the context from
      * @param contextSize context size to use building the context
-     * @param keys        list to store the keys
      * @param mask        mask for pulling out just the context bits
      */
-    private static void contextWith(final byte[] bases, final int contextSize, final ArrayList<Integer> keys, final int mask) {
+    private static ArrayList<Integer> contextWith(final byte[] bases, final int contextSize, final int mask) {
+
+        final int readLength = bases.length;
+        final ArrayList<Integer> keys = new ArrayList<Integer>(readLength);
 
         // the first contextSize-1 bases will not have enough previous context
-        for (int i = 1; i < contextSize && i <= bases.length; i++)
+        for (int i = 1; i < contextSize && i <= readLength; i++)
             keys.add(-1);
 
-        if (bases.length < contextSize)
+        if (readLength < contextSize)
-            return;
+            return keys;
 
         final int newBaseOffset = 2 * (contextSize - 1) + LENGTH_BITS;
 
@@ -171,7 +167,6 @@ public class ContextCovariate implements StandardCovariate {
             }
         }
 
-        final int readLength = bases.length;
         for (int currentIndex = contextSize; currentIndex < readLength; currentIndex++) {
             final int baseIndex = BaseUtils.simpleBaseToBaseIndex(bases[currentIndex]);
             if (baseIndex == -1) {                    // ignore non-ACGT bases
@@ -191,6 +186,8 @@ public class ContextCovariate implements StandardCovariate {
                 keys.add(-1);
             }
         }
+
+        return keys;
     }
 
     public static int keyFromContext(final String dna) {

public/java/src/org/broadinstitute/sting/utils/classloader/GATKLiteUtils.java

@@ -41,6 +41,21 @@ public class GATKLiteUtils {
      */
     private GATKLiteUtils() { }
 
+
+    private static Set<String> fullVersionGATKWalkers = new HashSet<String>();
+    static {
+        fullVersionGATKWalkers.add("HaplotypeCaller");
+        fullVersionGATKWalkers.add("ReduceReads");
+    }
+    /**
+     * Utility method to check whether a given walker is only available in the full GATK release
+     *
+     * @param walkerName   the walker class name (not the package) to check
+     */
+    public static boolean isAvailableOnlyInFullGATK(final String walkerName) {
+        return fullVersionGATKWalkers.contains(walkerName);
+    }
+
     /**
      * Utility method to determine whether this is the lite version of the GATK
      */

public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java

@@ -80,7 +80,7 @@ public class UserException extends ReviewedStingException {
 
     public static class NotSupportedInGATKLite extends UserException {
         public NotSupportedInGATKLite(String message) {
-            super(String.format("GATK Lite does support all of the features of the full version: %s", message));
+            super(String.format("GATK Lite does not support all of the features of the full version: %s", message));
         }
     }