HaplotypeCaller: fix -contamination argument, and add tests (#4455)

The -contamination argument was not hooked up properly in the HaplotypeCaller.
This patch fixes the tool argument, and adds tests on artificially contaminated
data to demonstrate that the feature works as intended.

Resolves #4312

Author: droazen

src/main/java/org/broadinstitute/hellbender/tools/walkers/genotyper/StandardCallerArgumentCollection.java

@@ -72,37 +72,75 @@ public void copyStandardCallerArgsFrom( final StandardCallerArgumentCollection o
     @Argument(fullName = "contamination-fraction-per-sample-file", shortName = "contamination-file", doc = "Tab-separated File containing fraction of contamination in sequencing data (per sample) to aggressively remove. Format should be \"<SampleID><TAB><Contamination>\" (Contamination is double) per line; No header.", optional = true)
     public File CONTAMINATION_FRACTION_FILE = null;
 
+    private DefaultedMap<String,Double> sampleContamination;
+    private boolean mapHasContaminationSet = false;
+
     /**
      * Returns true if there is some sample contamination present, false otherwise.
      * @return {@code true} iff there is some sample contamination
      */
     public boolean isSampleContaminationPresent() {
-        return (!Double.isNaN(CONTAMINATION_FRACTION) && CONTAMINATION_FRACTION > 0.0) || (sampleContamination != null && !sampleContamination.isEmpty());
+        return contaminationFractionIsSet(CONTAMINATION_FRACTION) || mapHasContaminationSet;
     }
-
-    private DefaultedMap<String,Double> sampleContamination;
-
+    
     /**
      * Returns an unmodifiable view of the map of SampleId -> contamination.
+     * 
+     * The returned map will return a default value equal to the configured
+     * {@link #CONTAMINATION_FRACTION} for samples whose contamination is not
+     * explicitly set.
      */
     public Map<String,Double> getSampleContamination() {
+        if (sampleContamination == null) {
+            setSampleContamination(Collections.emptyMap()); // default to empty map
+        }
         return Collections.unmodifiableMap(sampleContamination);
     }
 
     /**
-     * Returns the sample contamination or CONTAMINATION_FRACTION if no contamination level was specified for this sample.
+     * Set the sample contamination map using the provided map. The resulting map will have
+     * its default value for unknown keys set equal to {@link #CONTAMINATION_FRACTION}, regardless
+     * of any default value set in the provided map (if it's a DefaultedMap).
+     *
+     * @param sampleContamination Map of sample to contamination fraction with which to initialize our
+     *                            sample contamination map. Replaces any existing values in our map.
+     *                            The resulting map will have {@link #CONTAMINATION_FRACTION} as the default
+     *                            value for unknown keys, regardless of any default set in the provided map.
      */
-    public Double getSampleContamination(final String sampleId){
-        Utils.nonNull(sampleId);
-        if (sampleContamination == null){
-            setSampleContamination(new DefaultedMap<>(CONTAMINATION_FRACTION));//default to empty map
-        }
-        return sampleContamination.get(sampleId);
-    }
-
-    public void setSampleContamination(final DefaultedMap<String, Double> sampleContamination) {
+    public void setSampleContamination(final Map<String, Double> sampleContamination) {
         this.sampleContamination = new DefaultedMap<>(CONTAMINATION_FRACTION);  //NOTE: a bit weird because it ignores the default from the argument and uses ours
         this.sampleContamination.putAll(sampleContamination);                   //make a copy to be safe
+
+        this.mapHasContaminationSet = contaminationIsPresentInMap(this.sampleContamination);
+    }
+
+    /**
+     * @param fraction double value to test
+     * @return True if fraction represents non-zero contamination, otherwise false
+     */
+    private boolean contaminationFractionIsSet(final double fraction) {
+        return ! Double.isNaN(fraction) && fraction > 0.0;
+    }
+
+    /**
+     * Given a map of sample to contamination fraction, determines whether any samples have
+     * a non-zero contamination fraction set.
+     *
+     * @param contaminationMap sample -> contamination fraction map to test
+     * @return true if at least one sample has a non-zero contamination fraction, otherwise false
+     */
+    private boolean contaminationIsPresentInMap(final Map<String, Double> contaminationMap) {
+        if ( contaminationMap == null ) {
+            return false;
+        }
+
+        for ( final Map.Entry<String,Double> mapEntry : contaminationMap.entrySet() ) {
+            if ( contaminationFractionIsSet(mapEntry.getValue()) ) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     /**

src/test/java/org/broadinstitute/hellbender/tools/walkers/genotyper/StandardCallerArgumentCollectionUnitTest.java

@@ -1,22 +1,21 @@
 package org.broadinstitute.hellbender.tools.walkers.genotyper;
 
+import org.broadinstitute.hellbender.GATKBaseTest;
 import org.broadinstitute.hellbender.utils.Utils;
 import org.testng.Assert;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Random;
+import java.util.*;
 
 /**
- * Checks on Caller argument collection cloning.
+ * Checks on Caller argument collection cloning and getting/setting of values.
  *
  * @author Valentin Ruano-Rubio <[email protected]>
  */
-public final class StandardCallerArgumentCollectionUnitTest {
+public final class StandardCallerArgumentCollectionUnitTest extends GATKBaseTest {
 
     public final static List<Class<? extends StandardCallerArgumentCollection>> COLLECTION_CLASSES;
 
@@ -113,4 +112,91 @@ else if (type == Long.class)
         else
             return type;
     }
+
+    @Test
+    public void testGetSampleContaminationUninitializedArgs() {
+        final StandardCallerArgumentCollection args = new StandardCallerArgumentCollection();
+
+        Assert.assertFalse(args.isSampleContaminationPresent());
+        Map<String, Double> returnedContaminationMap = args.getSampleContamination();
+
+        // The returned map should be officially empty, but return 0.0 on query for any sample (it's a DefaultedMap)
+        Assert.assertTrue(returnedContaminationMap.isEmpty());
+        Assert.assertEquals(returnedContaminationMap.get("MySample"), 0.0);
+    }
+
+    @Test
+    public void testGetSampleContaminationInitializedFraction() {
+        final StandardCallerArgumentCollection args = new StandardCallerArgumentCollection();
+
+        args.CONTAMINATION_FRACTION = 0.1;
+        Assert.assertTrue(args.isSampleContaminationPresent());
+        Map<String, Double> returnedContaminationMap = args.getSampleContamination();
+
+        // The returned map should be officially empty, but return 0.1 on query for any sample (it's a DefaultedMap)
+        Assert.assertTrue(returnedContaminationMap.isEmpty());
+        Assert.assertEquals(returnedContaminationMap.get("MySample"), 0.1);
+    }
+
+    @Test
+    public void testGetSampleContaminationInitializedMap() {
+        final StandardCallerArgumentCollection args = new StandardCallerArgumentCollection();
+
+        Map<String,Double> contaminationMap = new HashMap<>();
+        contaminationMap.put("Sample1", 0.1);
+        contaminationMap.put("Sample2", 0.2);
+        args.setSampleContamination(contaminationMap);
+
+        Assert.assertTrue(args.isSampleContaminationPresent());
+        Map<String, Double> returnedContaminationMap = args.getSampleContamination();
+        
+        // The returned map should be of size 2, and return 0.0 on query for unknown samples (it's a DefaultedMap)
+        Assert.assertEquals(returnedContaminationMap.size(), 2);
+        Assert.assertEquals(returnedContaminationMap.get("Sample1"), 0.1);
+        Assert.assertEquals(returnedContaminationMap.get("Sample2"), 0.2);
+        Assert.assertEquals(returnedContaminationMap.get("Sample3"), 0.0);
+        Assert.assertEquals(returnedContaminationMap.get("Sample4"), 0.0);
+    }
+
+    @Test
+    public void testGetSampleContaminationInitializedMapAndFraction() {
+        final StandardCallerArgumentCollection args = new StandardCallerArgumentCollection();
+
+        args.CONTAMINATION_FRACTION = 0.05;
+        Map<String,Double> contaminationMap = new HashMap<>();
+        contaminationMap.put("Sample1", 0.1);
+        contaminationMap.put("Sample2", 0.2);
+        args.setSampleContamination(contaminationMap);
+
+        Assert.assertTrue(args.isSampleContaminationPresent());
+        Map<String, Double> returnedContaminationMap = args.getSampleContamination();
+
+        // The returned map should be of size 2, and return 0.05 on query for unknown samples (it's a DefaultedMap)
+        Assert.assertEquals(returnedContaminationMap.size(), 2);
+        Assert.assertEquals(returnedContaminationMap.get("Sample1"), 0.1);
+        Assert.assertEquals(returnedContaminationMap.get("Sample2"), 0.2);
+        Assert.assertEquals(returnedContaminationMap.get("Sample3"), 0.05);
+        Assert.assertEquals(returnedContaminationMap.get("Sample4"), 0.05);
+    }
+
+    @Test
+    public void testGetSampleContaminationMapWithNoContamination() {
+        final StandardCallerArgumentCollection args = new StandardCallerArgumentCollection();
+
+        // Create a map that doesn't actually have any contamination set
+        Map<String,Double> contaminationMap = new HashMap<>();
+        contaminationMap.put("Sample1", 0.0);
+        contaminationMap.put("Sample2", 0.0);
+        args.setSampleContamination(contaminationMap);
+
+        Assert.assertFalse(args.isSampleContaminationPresent());
+        Map<String, Double> returnedContaminationMap = args.getSampleContamination();
+
+        // The returned map should be of size 2, and return 0.0 on queries for any sample (it's a DefaultedMap)
+        Assert.assertEquals(returnedContaminationMap.size(), 2);
+        Assert.assertEquals(returnedContaminationMap.get("Sample1"), 0.0);
+        Assert.assertEquals(returnedContaminationMap.get("Sample2"), 0.0);
+        Assert.assertEquals(returnedContaminationMap.get("Sample3"), 0.0);
+        Assert.assertEquals(returnedContaminationMap.get("Sample4"), 0.0);
+    }
 }