check point review edits

src/main/java/org/broadinstitute/hellbender/tools/ApplyBQSRArgumentCollection.java

@@ -8,7 +8,6 @@
  */
 public class ApplyBQSRArgumentCollection extends ApplyBQSRUniqueArgumentCollection {
     private static final long serialVersionUID = 1L;
-    public static final String ALLOW_MISSING_READ_GROUPS_LONG_NAME = "allow-missing-read-group";
     public static final String USE_ORIGINAL_QUALITIES_LONG_NAME = "use-original-qualities";
 
     /**
@@ -30,12 +29,4 @@ public class ApplyBQSRArgumentCollection extends ApplyBQSRUniqueArgumentCollecti
      */
     @Argument(fullName=USE_ORIGINAL_QUALITIES_LONG_NAME, shortName = "OQ", doc = "Use the base quality scores from the OQ tag", optional = true)
     public Boolean useOriginalBaseQualities = false;
-
-    /**
-     * If set to true, do not throw an error upon encountering a read with a read group that's not in the recalibration table.
-     * Instead, simply set the quantized original base qualities as the recalibrated base qualities.
-     */ // tsato: should this be in the *unique* ApplyBQSRArgumentCollection?
-    @Argument(fullName = ALLOW_MISSING_READ_GROUPS_LONG_NAME, doc = "Do not throw an error when encountering a read group not in the recal table", optional = true)
-    public boolean allowMissingReadGroups = false;
-
 }

src/main/java/org/broadinstitute/hellbender/tools/ApplyBQSRUniqueArgumentCollection.java

@@ -18,7 +18,7 @@ public class ApplyBQSRUniqueArgumentCollection implements Serializable {
     private static final long serialVersionUID = 1L;
 
     public static final String STATIC_QUANTIZED_QUALS_LONG_NAME = "static-quantized-quals";
-
+    public static final String ALLOW_MISSING_READ_GROUPS_LONG_NAME = "allow-missing-read-group";
 
     /**
      * Turns on the base quantization module. It requires a recalibration report.
@@ -66,6 +66,13 @@ public class ApplyBQSRUniqueArgumentCollection implements Serializable {
     @Argument(fullName = "global-qscore-prior", doc = "Global Qscore Bayesian prior to use for BQSR", optional = true)
     public double globalQScorePrior = -1.0;
 
+    /**
+     * If set to true, do not throw an error upon encountering a read with a read group that's not in the recalibration table.
+     * Instead, simply set the quantized original base qualities as the recalibrated base qualities.
+     */
+    @Argument(fullName = ALLOW_MISSING_READ_GROUPS_LONG_NAME, doc = "Do not throw an error when encountering a read group not in the recal table", optional = true)
+    public boolean allowMissingReadGroups = false;
+
     /**
      * Combine the extra arguments in {@link ApplyBQSRArgumentCollection} that are not in this {@link ApplyBQSRUniqueArgumentCollection}
      * from the given {@link RecalibrationArgumentCollection} to create a {@link ApplyBQSRArgumentCollection}.
@@ -80,6 +87,8 @@ public ApplyBQSRArgumentCollection toApplyBQSRArgumentCollection(RecalibrationAr
         ret.roundDown = this.roundDown;
         ret.emitOriginalQuals = this.emitOriginalQuals;
         ret.globalQScorePrior = this.globalQScorePrior;
+        ret.allowMissingReadGroups = this.allowMissingReadGroups;
+
         // include all the fields from RecalibrationArgumentCollection
         ret.PRESERVE_QSCORES_LESS_THAN = bqsrArgs.PRESERVE_QSCORES_LESS_THAN;
         ret.useOriginalBaseQualities = bqsrArgs.useOriginalBaseQualities;

src/main/java/org/broadinstitute/hellbender/transformers/BQSRReadTransformer.java

@@ -51,7 +51,7 @@ public final class BQSRReadTransformer implements ReadTransformer {
     private byte[] staticQuantizedMapping;
     private final CovariateKeyCache keyCache;
 
-    private boolean allowMissingReadGroups;
+    private final boolean allowMissingReadGroups;
     private static final int READ_GROUP_MISSING_IN_RECAL_TABLE_CODE = -1;
 
     private List<Byte> quantizedQuals;
@@ -155,7 +155,7 @@ public GATKRead apply(final GATKRead originalRead) {
 
         final PerReadCovariateMatrix perReadCovariateMatrix = RecalUtils.computeCovariates(read, header, covariates, false, keyCache);
 
-        // clear indel qualities
+        // clear indel qualities TODO: do we still modify indel qualities?
         read.clearAttribute(ReadUtils.BQSR_BASE_INSERTION_QUALITIES);
         read.clearAttribute(ReadUtils.BQSR_BASE_DELETION_QUALITIES);
 

src/main/java/org/broadinstitute/hellbender/utils/recalibration/BaseRecalibrationEngine.java

@@ -329,7 +329,13 @@ private GATKRead setDefaultBaseQualities( final GATKRead read ) {
         return read;
     }
 
-    // TODO: add docs
+    /**
+     * Outputs a boolean array that has the same length as the read.
+     * The array contains true at each index if the position meets one of the following criteria:
+     *   1) not a regular base
+     *   2) base quality is less than 6
+     *   3) is a known site.
+     */
     private boolean[] calculateSkipArray( final GATKRead read, final Iterable<? extends Locatable> knownSites ) {
         final int readLength = read.getLength();
         final boolean[] skip = new boolean[readLength];
@@ -340,7 +346,10 @@ private boolean[] calculateSkipArray( final GATKRead read, final Iterable<? exte
         return skip;
     }
 
-    // TODO: add docs
+    /**
+     * Outputs a boolean array that has the same length as the read and contains true at positions where known events
+     * occur, as determined by the knownSites variable.
+     */
     private static boolean[] calculateKnownSites( final GATKRead read, final Iterable<? extends Locatable> knownSites ) {
         final int readLength = read.getLength();
         final boolean[] knownSitesArray = new boolean[readLength];//initializes to all false

src/main/java/org/broadinstitute/hellbender/utils/recalibration/RecalDatum.java

@@ -153,7 +153,7 @@ public double getEmpiricalErrorRate() {
             return doubleMismatches / doubleObservations;
         }
     }
-    // tsato: int-double
+
     public void setEmpiricalQuality(final int empiricalQuality) {
         if ( empiricalQuality < 0 ) throw new IllegalArgumentException("empiricalQuality < 0");
         if ( Double.isInfinite(empiricalQuality) ) throw new IllegalArgumentException("empiricalQuality is infinite");

src/main/java/org/broadinstitute/hellbender/utils/recalibration/RecalUtils.java

@@ -32,7 +32,7 @@
 public final class RecalUtils {
     public static final String ARGUMENT_REPORT_TABLE_TITLE = "Arguments";
     public static final String QUANTIZED_REPORT_TABLE_TITLE = "Quantized";
-    public static final String READGROUP_REPORT_TABLE_TITLE = "RecalTable0"; // TODO: make them more descriptive
+    public static final String READGROUP_REPORT_TABLE_TITLE = "RecalTable0";
     public static final String QUALITY_SCORE_REPORT_TABLE_TITLE = "RecalTable1";
     public static final String ALL_COVARIATES_REPORT_TABLE_TITLE = "RecalTable2";
 
@@ -56,7 +56,7 @@ public final class RecalUtils {
 
     private static final String SCRIPT_FILE = "BQSR.R";
     public static final int EMPIRICAL_QUAL_DECIMAL_PLACES = 4;
-    public static final int REPORTED_QUALITY_DECIMAL_PLACES = 4; // tsato: "estimated" q reported...we need to rename (DONE)
+    public static final int REPORTED_QUALITY_DECIMAL_PLACES = 4;
     public static final int NUMBER_ERRORS_DECIMAL_PLACES = 2;
 
     private static final Pair<String, String> covariateValue     = new MutablePair<>(RecalUtils.COVARIATE_VALUE_COLUMN_NAME, "%s");

src/main/java/org/broadinstitute/hellbender/utils/recalibration/covariates/ContextCovariate.java

@@ -21,7 +21,7 @@ public final class ContextCovariate implements Covariate {
     private static final long serialVersionUID = 1L;
     private static final Logger logger = LogManager.getLogger(ContextCovariate.class);
 
-    private final int mismatchesContextSize; // TODO: rename "mismatch" here
+    private final int mismatchesContextSize;
     private final int indelsContextSize;
 
     private final int mismatchesKeyMask;