diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java index f2423da33..08cc4b442 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java @@ -269,7 +269,9 @@ public class VariantEvalWalker extends RodWalker implements Tr // Initialize the set of stratifications and evaluations to use // The list of stratifiers and evaluators to use final List stratificationObjects = variantEvalUtils.initializeStratificationObjects(NO_STANDARD_STRATIFICATIONS, STRATIFICATIONS_TO_USE); - final Set> evaluationObjects = variantEvalUtils.initializeEvaluationObjects(NO_STANDARD_MODULES, MODULES_TO_USE); + final Set> evaluationClasses = variantEvalUtils.initializeEvaluationObjects(NO_STANDARD_MODULES, MODULES_TO_USE); + + checkForIncompatibleEvaluatorsAndStratifiers(stratificationObjects, evaluationClasses); for ( VariantStratifier vs : stratificationObjects ) { if ( vs.getName().equals("Filter") ) @@ -289,10 +291,10 @@ public class VariantEvalWalker extends RodWalker implements Tr } // Initialize the evaluation contexts - createStratificationStates(stratificationObjects, evaluationObjects); + createStratificationStates(stratificationObjects, evaluationClasses); // Initialize report table - report = variantEvalUtils.initializeGATKReport(stratificationObjects, evaluationObjects); + report = variantEvalUtils.initializeGATKReport(stratificationObjects, evaluationClasses); // Load ancestral alignments if (ancestralAlignmentsFile != null) { @@ -309,6 +311,19 @@ public class VariantEvalWalker extends RodWalker implements Tr } } + final void checkForIncompatibleEvaluatorsAndStratifiers( final List stratificationObjects, + Set> evaluationClasses) { + for ( final VariantStratifier vs : stratificationObjects ) { + for ( Class ec : evaluationClasses ) + if ( vs.getIncompatibleEvaluators().contains(ec) ) + throw new UserException.BadArgumentValue("ST and ET", + "The selected stratification " + vs.getName() + + " and evaluator " + ec.getSimpleName() + + " are incompatible due to combinatorial memory requirements." + + " Please disable one"); + } + } + final void createStratificationStates(final List stratificationObjects, final Set> evaluationObjects) { final List strats = new ArrayList(stratificationObjects); stratManager = new StratificationManager(strats); diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/AlleleCount.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/AlleleCount.java index 1068b2cc8..319ab96b2 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/AlleleCount.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/AlleleCount.java @@ -3,13 +3,13 @@ package org.broadinstitute.sting.gatk.walkers.varianteval.stratifications; import org.broadinstitute.sting.commandline.RodBinding; import org.broadinstitute.sting.gatk.contexts.ReferenceContext; import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker; +import org.broadinstitute.sting.gatk.walkers.varianteval.evaluators.VariantEvaluator; +import org.broadinstitute.sting.gatk.walkers.varianteval.evaluators.VariantSummary; import org.broadinstitute.sting.utils.exceptions.UserException; import org.broadinstitute.sting.utils.variantcontext.Allele; import org.broadinstitute.sting.utils.variantcontext.VariantContext; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.util.*; /** * Stratifies the eval RODs by the allele count of the alternate allele @@ -50,9 +50,14 @@ public class AlleleCount extends VariantStratifier { } else // by default, the site is considered monomorphic AC = 0; - return Collections.singletonList((Object)AC); + return Collections.singletonList((Object) AC); } else { return Collections.emptyList(); } } + + @Override + public Set> getIncompatibleEvaluators() { + return new HashSet>(Arrays.asList(VariantSummary.class)); + } } diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/Sample.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/Sample.java index d78a35b40..621f4337f 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/Sample.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/Sample.java @@ -2,11 +2,11 @@ package org.broadinstitute.sting.gatk.walkers.varianteval.stratifications; import org.broadinstitute.sting.gatk.contexts.ReferenceContext; import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker; +import org.broadinstitute.sting.gatk.walkers.varianteval.evaluators.VariantEvaluator; +import org.broadinstitute.sting.gatk.walkers.varianteval.evaluators.VariantSummary; import org.broadinstitute.sting.utils.variantcontext.VariantContext; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; +import java.util.*; /** * Stratifies the eval RODs by each sample in the eval ROD. @@ -22,6 +22,11 @@ public class Sample extends VariantStratifier { } public List getRelevantStates(ReferenceContext ref, RefMetaDataTracker tracker, VariantContext comp, String compName, VariantContext eval, String evalName, String sampleName) { - return Collections.singletonList((Object)sampleName); + return Collections.singletonList((Object) sampleName); + } + + @Override + public Set> getIncompatibleEvaluators() { + return new HashSet>(Arrays.asList(VariantSummary.class)); } } diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/VariantStratifier.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/VariantStratifier.java index 702a10b3d..ec902704e 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/VariantStratifier.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/VariantStratifier.java @@ -3,11 +3,14 @@ package org.broadinstitute.sting.gatk.walkers.varianteval.stratifications; import org.broadinstitute.sting.gatk.contexts.ReferenceContext; import org.broadinstitute.sting.gatk.refdata.RefMetaDataTracker; import org.broadinstitute.sting.gatk.walkers.varianteval.VariantEvalWalker; +import org.broadinstitute.sting.gatk.walkers.varianteval.evaluators.VariantEvaluator; import org.broadinstitute.sting.gatk.walkers.varianteval.stratifications.manager.Stratifier; import org.broadinstitute.sting.utils.variantcontext.VariantContext; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Set; public abstract class VariantStratifier implements Comparable, Stratifier { private VariantEvalWalker variantEvalWalker; @@ -65,4 +68,16 @@ public abstract class VariantStratifier implements Comparable public final ArrayList getAllStates() { return states; } + + + /** + * The way for a stratifier to specify that it's incompatible with specific evaluations. For + * example, VariantSummary includes a per-sample metric, and so cannot be used safely with Sample + * or AlleleCount stratifications as this introduces an O(n^2) memory and cpu cost. + * + * @return the set of VariantEvaluators that cannot be active with this Stratification + */ + public Set> getIncompatibleEvaluators() { + return Collections.emptySet(); + } } diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratNode.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratNode.java index 6b3375048..2bcb20e8e 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratNode.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratNode.java @@ -62,8 +62,7 @@ import java.util.*; class StratNode implements Iterable> { int key = -1; final T stratifier; - // TODO -- track state key that maps to root node - final Map> subnodes; + final Map> subnodes; // NOTE, because we don't iterate our best option is a HashMap protected StratNode() { this.subnodes = Collections.emptyMap(); @@ -72,7 +71,8 @@ class StratNode implements Iterable> { protected StratNode(final T stratifier, final Map> subnodes) { this.stratifier = stratifier; - this.subnodes = subnodes; + // important to reallocate an unmodififable hashmap with this specific size for space and safety + this.subnodes = Collections.unmodifiableMap(new HashMap>(subnodes)); } @Requires("key >= 0") diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratificationManager.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratificationManager.java index a2653584e..86821fbc1 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratificationManager.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/stratifications/manager/StratificationManager.java @@ -118,7 +118,7 @@ public class StratificationManager implements Map(states)); + stratifierValuesByKey.set(node.getKey(), Collections.unmodifiableList(new ArrayList(states))); } else { for ( Map.Entry> entry : node.getSubnodes().entrySet() ) { final LinkedList newStates = new LinkedList(states); diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java index 610733d9c..14bf24b29 100755 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalIntegrationTest.java @@ -25,6 +25,7 @@ package org.broadinstitute.sting.gatk.walkers.varianteval; import org.broadinstitute.sting.WalkerTest; +import org.broadinstitute.sting.utils.exceptions.UserException; import org.testng.annotations.Test; import java.util.Arrays; @@ -491,4 +492,18 @@ public class VariantEvalIntegrationTest extends WalkerTest { ); executeTest("testModernVCFWithLargeIndels", spec); } + + @Test() + public void testIncompatibleEvalAndStrat() { + WalkerTestSpec spec = new WalkerTestSpec( + buildCommandLine( + "-T VariantEval", + "-R " + b37KGReference, + "-eval " + validationDataLocation + "/NA12878.HiSeq.WGS.b37_decoy.indel.recalibrated.vcf", + "-L 20 -noST -ST AlleleCount -noEV -EV VariantSummary" + ), + 0, + UserException.class); + executeTest("testIncompatibleEvalAndStrat", spec); + } }