From beb11af1d4bd8924d760f93567d42e0f99537493 Mon Sep 17 00:00:00 2001 From: Laura Gauthier Date: Fri, 2 Aug 2019 11:35:35 -0400 Subject: [PATCH 1/6] Item #3 -- "Address my maintenance concerns about AnnotationUtils.isAlleleSpecific()" --- .../tools/walkers/annotator/AnnotationUtils.java | 11 +---------- .../annotator/allelespecific/AS_InbreedingCoeff.java | 2 +- .../annotator/allelespecific/AS_QualByDepth.java | 2 +- .../allelespecific/AS_RMSMappingQuality.java | 2 +- .../annotator/allelespecific/AS_RankSumTest.java | 2 +- .../annotator/allelespecific/AS_StrandBiasTest.java | 2 +- 6 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java index 124045e9bd6..b167173bd93 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java @@ -50,16 +50,7 @@ public static String encodeAnyASList( final List somethingList) { * @return true if the annotation is expected to have values per-allele */ public static boolean isAlleleSpecific(final InfoFieldAnnotation annotation) { - if (annotation instanceof AS_RankSumTest) { - return true; - } - if (annotation instanceof AS_StrandBiasTest) { - return true; - } - if (annotation instanceof AS_RMSMappingQuality) { - return true; - } - if (annotation instanceof AS_StandardAnnotation) { + if (annotation instanceof AlleleSpecificAnnotation) { return true; } return false; diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_InbreedingCoeff.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_InbreedingCoeff.java index 886397e7062..a3bb4f229bd 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_InbreedingCoeff.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_InbreedingCoeff.java @@ -40,7 +40,7 @@ */ //TODO: this can't extend InbreedingCoeff because that one is Standard and it would force this to be output all the time; should fix code duplication nonetheless @DocumentedFeature(groupName=HelpConstants.DOC_CAT_ANNOTATORS, groupSummary=HelpConstants.DOC_CAT_ANNOTATORS_SUMMARY, summary="Allele-specific likelihood-based test for the consanguinity among samples (AS_InbreedingCoeff)") -public final class AS_InbreedingCoeff extends InfoFieldAnnotation implements AS_StandardAnnotation { +public final class AS_InbreedingCoeff extends InfoFieldAnnotation implements AS_StandardAnnotation, AlleleSpecificAnnotation { public static final int MIN_SAMPLES = 10; private Set founderIds; //TODO: either use this or enter a bug report diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_QualByDepth.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_QualByDepth.java index 42066f1eecd..e3d21b64444 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_QualByDepth.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_QualByDepth.java @@ -46,7 +46,7 @@ * */ @DocumentedFeature(groupName=HelpConstants.DOC_CAT_ANNOTATORS, groupSummary=HelpConstants.DOC_CAT_ANNOTATORS_SUMMARY, summary="Allele-specific call confidence normalized by depth of sample reads supporting the allele (AS_QD)") -public class AS_QualByDepth extends InfoFieldAnnotation implements ReducibleAnnotation, AS_StandardAnnotation { +public class AS_QualByDepth extends InfoFieldAnnotation implements ReducibleAnnotation, AS_StandardAnnotation, AlleleSpecificAnnotation { @Override public List getKeyNames() { return Arrays.asList(GATKVCFConstants.AS_QUAL_BY_DEPTH_KEY); } diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RMSMappingQuality.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RMSMappingQuality.java index 5eaf0ed8d28..bd88f204fa0 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RMSMappingQuality.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RMSMappingQuality.java @@ -43,7 +43,7 @@ * */ @DocumentedFeature(groupName=HelpConstants.DOC_CAT_ANNOTATORS, groupSummary=HelpConstants.DOC_CAT_ANNOTATORS_SUMMARY, summary="Allele-specific root-mean-square of the mapping quality of reads across all samples (AS_MQ)") -public final class AS_RMSMappingQuality extends InfoFieldAnnotation implements AS_StandardAnnotation, ReducibleAnnotation { +public final class AS_RMSMappingQuality extends InfoFieldAnnotation implements AS_StandardAnnotation, ReducibleAnnotation, AlleleSpecificAnnotation { private final String printFormat = "%.2f"; diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RankSumTest.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RankSumTest.java index 77559e6197f..4565d5e5aef 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RankSumTest.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_RankSumTest.java @@ -21,7 +21,7 @@ /** * Allele-specific implementation of rank sum test annotations */ -public abstract class AS_RankSumTest extends RankSumTest implements ReducibleAnnotation { +public abstract class AS_RankSumTest extends RankSumTest implements ReducibleAnnotation, AlleleSpecificAnnotation { private static final Logger logger = LogManager.getLogger(AS_RankSumTest.class); public static final String RAW_DELIM = ","; diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_StrandBiasTest.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_StrandBiasTest.java index d4fa7cd33bd..2ab6eba625d 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_StrandBiasTest.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AS_StrandBiasTest.java @@ -18,7 +18,7 @@ /** * Allele-specific implementation of strand bias annotations */ -public abstract class AS_StrandBiasTest extends StrandBiasTest implements ReducibleAnnotation { +public abstract class AS_StrandBiasTest extends StrandBiasTest implements ReducibleAnnotation, AlleleSpecificAnnotation { private final static Logger logger = LogManager.getLogger(AS_StrandBiasTest.class); public static final String SPLIT_DELIM = "\\|"; //String.split takes a regex, so we need to escape the pipe public static final String PRINT_DELIM = "|"; From 0ce6686fca1f088e9eeaac0cc758c9b814cb6739 Mon Sep 17 00:00:00 2001 From: Laura Gauthier Date: Fri, 2 Aug 2019 12:33:42 -0400 Subject: [PATCH 2/6] Item #2 -- "Add a simple direct integration test for the new --floor-blocks HaplotypeCaller arg" Also clarified beta status for GVCF mode just for Mutect2 --- ...AssemblyBasedCallerArgumentCollection.java | 8 ++-- .../HaplotypeCallerEngine.java | 12 ++++++ .../HaplotypeCallerIntegrationTest.java | 38 +++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java index 4464d26f763..dae4d5392fd 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java @@ -25,6 +25,7 @@ public abstract class AssemblyBasedCallerArgumentCollection { public static final String MIN_BASE_QUALITY_SCORE_LONG_NAME = "min-base-quality-score"; public static final String SMITH_WATERMAN_LONG_NAME = "smith-waterman"; + public static final String EMIT_REFERENCE_CONFIDENCE_LONG_NAME = "emit-ref-confidence"; public ReadThreadingAssembler createReadThreadingAssembler() { final ReadThreadingAssembler assemblyEngine = assemblerArgs.makeReadThreadingAssembler(); @@ -107,11 +108,12 @@ public ReadThreadingAssembler createReadThreadingAssembler() { public SmithWatermanAligner.Implementation smithWatermanImplementation = SmithWatermanAligner.Implementation.JAVA; /** - * (BETA feature) The reference confidence mode makes it possible to emit a per-bp or summarized confidence estimate for a site being strictly homozygous-reference. - * This is similar to the HaplotypeCaller reference confidence/GVCF mode. See https://software.broadinstitute.org/gatk/documentation/article.php?id=4017 for information about GVCFs. + * The reference confidence mode makes it possible to emit a per-bp or summarized confidence estimate for a site being strictly homozygous-reference. + * See https://software.broadinstitute.org/gatk/documentation/article.php?id=4017 for information about GVCFs. + * This is a BETA FEATURE for Mutect2 similar to the HaplotypeCaller reference confidence/GVCF mode. */ @Advanced - @Argument(fullName="emit-ref-confidence", shortName="ERC", doc="(BETA feature) Mode for emitting reference confidence scores", optional = true) + @Argument(fullName=EMIT_REFERENCE_CONFIDENCE_LONG_NAME, shortName="ERC", doc="Mode for emitting reference confidence scores (BETA for Mutect2)", optional = true) public ReferenceConfidenceMode emitReferenceConfidence = ReferenceConfidenceMode.NONE; protected abstract int getDefaultMaxMnpDistance(); diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java index 6f6564f34fc..161453c0962 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java @@ -273,6 +273,18 @@ private void validateAndInitializeArgs() { if ( emitReferenceConfidence() && samplesList.numberOfSamples() != 1 ) { throw new CommandLineException.BadArgumentValue("--emit-ref-confidence", "Can only be used in single sample mode currently. Use the --sample-name argument to run on a single sample out of a multi-sample BAM file."); } + + if (hcArgs.floorBlocks && !emitReferenceConfidence()) { + throw new UserException(HaplotypeCallerArgumentCollection.OUTPUT_BLOCK_LOWER_BOUNDS + " refers to GVCF blocks," + + " so reference confidence mode (" + AssemblyBasedCallerArgumentCollection.EMIT_REFERENCE_CONFIDENCE_LONG_NAME + + ") must be specified."); + } + + if (hcArgs.floorBlocks && !emitReferenceConfidence()) { + throw new UserException(HaplotypeCallerArgumentCollection.OUTPUT_BLOCK_LOWER_BOUNDS + " refers to GVCF blocks," + + " so reference confidence mode (" + AssemblyBasedCallerArgumentCollection.EMIT_REFERENCE_CONFIDENCE_LONG_NAME + + ") must be specified."); + } } private void initializeSamples() { diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java index e6042176622..93ba3cd9609 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java @@ -13,6 +13,7 @@ import org.apache.commons.lang3.tuple.Pair; import org.broadinstitute.barclay.argparser.CommandLineException; import org.broadinstitute.hellbender.CommandLineProgramTest; +import org.broadinstitute.hellbender.GATKBaseTest; import org.broadinstitute.hellbender.cmdline.StandardArgumentDefinitions; import org.broadinstitute.hellbender.cmdline.argumentcollections.IntervalArgumentCollection; import org.broadinstitute.hellbender.engine.AssemblyRegionWalker; @@ -371,6 +372,43 @@ else if (!vc.getAlternateAllele(0).equals(Allele.NON_REF_ALLELE)){ //there } } + /* + * Test that in VCF mode we're consistent with past GATK4 results + */ + @Test(dataProvider="HaplotypeCallerTestInputs") + public void testFloorGVCFBlocks(final String inputFileName, final String referenceFileName) throws Exception { + Utils.resetRandomGenerator(); + + final File output = createTempFile("testFloorGVCFBlocks", ".vcf"); + + final List requestedGqBands = Arrays.asList("10","20","30","40","50","60"); + + final ArgumentsBuilder args = new ArgumentsBuilder().addInput(new File(inputFileName)) + .addReference(new File(referenceFileName)) + .addInterval(new SimpleInterval("20:10000000-10100000")) + .addBooleanArgument(StandardArgumentDefinitions.ADD_OUTPUT_VCF_COMMANDLINE, false) + .addArgument("pairHMM", "AVX_LOGLESS_CACHING") + .addArgument("floor-blocks") + .addArgument("ERC", "GVCF") + .addOutput(output); + requestedGqBands.forEach(x -> args.addArgument("GQB",x)); + runCommandLine(args); + + final List allGqBands = new ArrayList(requestedGqBands); + allGqBands.add("99"); + allGqBands.add("0"); + + //The interval here is big, so use a FeatureDataSource to limit memory usage + try (final FeatureDataSource actualVcs = new FeatureDataSource<>(output)) { + actualVcs.forEach(vc -> { + //sometimes there are calls with alt alleles that are genotyped hom ref and those don't get floored + if (vc.hasAttribute("END") && vc.getGenotype(0).hasGQ()) { + Assert.assertTrue(allGqBands.contains(Integer.toString(vc.getGenotype(0).getGQ()))); + } + }); + } + } + @Test public void testGenotypeGivenAllelesMode() throws IOException { Utils.resetRandomGenerator(); From 2f65547161f62c0e82886b608b599873925aec2e Mon Sep 17 00:00:00 2001 From: Laura Gauthier Date: Fri, 2 Aug 2019 13:28:58 -0400 Subject: [PATCH 3/6] Item #1 -- remove `instanceof` call and let every GATKTool have default GenomicsDB options --- .../hellbender/engine/FeatureManager.java | 10 +++++----- .../org/broadinstitute/hellbender/engine/GATKTool.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java b/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java index 7aaa1e8507c..38f606b7625 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java @@ -147,14 +147,14 @@ public FeatureManager(final CommandLineProgram toolInstance, final int featureQu * the end of query intervals in anticipation of future queries (>= 0). * @param cloudPrefetchBuffer MB size of caching/prefetching wrapper for the data, if on Google Cloud (0 to disable). * @param cloudIndexPrefetchBuffer MB size of caching/prefetching wrapper for the index, if on Google Cloud (0 to disable). - * @param reference reference to use when opening feature files, may be null, currently only used by Genomics DB + * @param gdbOptions settings for GenomicsDB to use when reading from a GenomicsDB workspace * */ - public FeatureManager(final CommandLineProgram toolInstance, final int featureQueryLookahead, final int cloudPrefetchBuffer, final int cloudIndexPrefetchBuffer, final Path reference) { + public FeatureManager(final CommandLineProgram toolInstance, final int featureQueryLookahead, final int cloudPrefetchBuffer, final int cloudIndexPrefetchBuffer, final GenomicsDBOptions gdbOptions) { this.toolInstanceSimpleClassName = toolInstance.getClass().getSimpleName(); this.featureSources = new LinkedHashMap<>(); - initializeFeatureSources(featureQueryLookahead, toolInstance, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, reference); + initializeFeatureSources(featureQueryLookahead, toolInstance, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, gdbOptions); } /** @@ -193,7 +193,7 @@ public FeatureManager(final CommandLineProgram toolInstance, final int featureQu * @param cloudIndexPrefetchBuffer MB size of caching/prefetching wrapper for the index, if on Google Cloud (0 to disable). */ @SuppressWarnings({"unchecked", "rawtypes"}) - private void initializeFeatureSources( final int featureQueryLookahead, final CommandLineProgram toolInstance, final int cloudPrefetchBuffer, final int cloudIndexPrefetchBuffer, final Path reference) { + private void initializeFeatureSources( final int featureQueryLookahead, final CommandLineProgram toolInstance, final int cloudPrefetchBuffer, final int cloudIndexPrefetchBuffer, final GenomicsDBOptions gdbOptions) { // Discover all arguments of type FeatureInput (or Collections thereof) in our tool's class hierarchy // (and associated ArgumentCollections). Arguments not specified by the user on the command line will @@ -208,7 +208,7 @@ private void initializeFeatureSources( final int featureQueryLookahead, final Co if ( featureInput != null ) { final Class featureType = getFeatureTypeForFeatureInputField(featureArgument.getKey()); addToFeatureSources(featureQueryLookahead, featureInput, featureType, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - toolInstance instanceof VariantWalker ? ((VariantWalker) toolInstance).getGenomicsDBOptions() : null); + gdbOptions); } } } diff --git a/src/main/java/org/broadinstitute/hellbender/engine/GATKTool.java b/src/main/java/org/broadinstitute/hellbender/engine/GATKTool.java index 654d9b851d1..b90225d7427 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/GATKTool.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/GATKTool.java @@ -415,11 +415,11 @@ protected List transformTraversalIntervals(final List Date: Fri, 2 Aug 2019 13:37:24 -0400 Subject: [PATCH 4/6] If I were smarter, IntelliJ refactoring coult have done this --- .../hellbender/engine/FeatureContext.java | 6 +++--- .../hellbender/engine/FeatureManager.java | 16 +++++++--------- .../hellbender/engine/FeatureWalker.java | 2 +- .../hellbender/engine/IntervalWalker.java | 2 +- .../hellbender/engine/ReadWalker.java | 2 +- .../hellbender/engine/VariantWalkerBase.java | 2 +- .../tools/copynumber/AnnotateIntervals.java | 2 +- .../engine/DummyPlaceholderGatkTool.java | 2 +- .../utils/test/FuncotatorTestUtils.java | 6 +++--- 9 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/engine/FeatureContext.java b/src/main/java/org/broadinstitute/hellbender/engine/FeatureContext.java index 53ed144b5e2..0a8f6b2599b 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/FeatureContext.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/FeatureContext.java @@ -310,9 +310,9 @@ private List subsetToStartPosition(final Collection fe * @param interval genomic interval for the result. Typically, this would be the interval of the variant. Never {@link null}. * @param featureQueryLookahead When querying FeatureDataSources, cache this many extra bases of context beyond * the end of query intervals in anticipation of future queries. Must be >= 0. If uncertain, use zero. - * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use zero. - * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use zero. - * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use {@code null}. + * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use zero. + * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use zero. + * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use {@code null}. */ @VisibleForTesting public static FeatureContext createFeatureContextForTesting(final Map, Class> featureInputsWithType, final String dummyToolInstanceName, diff --git a/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java b/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java index 38f606b7625..2283d783485 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/FeatureManager.java @@ -19,7 +19,6 @@ import org.broadinstitute.hellbender.utils.Utils; import org.broadinstitute.hellbender.utils.config.ConfigFactory; import org.broadinstitute.hellbender.utils.config.GATKConfig; -import org.broadinstitute.hellbender.utils.variant.GATKVCFConstants; import java.io.File; import java.lang.reflect.Field; @@ -140,8 +139,7 @@ public FeatureManager(final CommandLineProgram toolInstance, final int featureQu * Create a FeatureManager given a CommandLineProgram tool instance, discovering all FeatureInput * arguments in the tool and creating query-able FeatureDataSources for them. Allows control over * how much caching is performed by each {@link FeatureDataSource}. - * - * @param toolInstance Instance of the tool to be run (potentially containing one or more FeatureInput arguments) + * @param toolInstance Instance of the tool to be run (potentially containing one or more FeatureInput arguments) * Must have undergone command-line argument parsing and argument value injection already. * @param featureQueryLookahead When querying FeatureDataSources, cache this many extra bases of context beyond * the end of query intervals in anticipation of future queries (>= 0). @@ -158,17 +156,17 @@ public FeatureManager(final CommandLineProgram toolInstance, final int featureQu } /** - * Same as {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)}, except used when the + * Same as {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)}, except used when the * FeatureInputs (and associated types) are known. * * This constructor should only be used in test code. * * @param featureInputsToTypeMap {@link Map} of a {@link FeatureInput} to the output type that must extend {@link Feature}. Never {@code null} - * @param toolInstanceName See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} - * @param featureQueryLookahead See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} - * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} - * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} - * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} + * @param toolInstanceName See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)} + * @param featureQueryLookahead See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)} + * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)} + * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)} + * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, GenomicsDBOptions)} */ @VisibleForTesting FeatureManager(final Map, Class> featureInputsToTypeMap, final String toolInstanceName, final int featureQueryLookahead, final int cloudPrefetchBuffer, final int cloudIndexPrefetchBuffer, final Path reference) { diff --git a/src/main/java/org/broadinstitute/hellbender/engine/FeatureWalker.java b/src/main/java/org/broadinstitute/hellbender/engine/FeatureWalker.java index 1246f03c106..6aab39207df 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/FeatureWalker.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/FeatureWalker.java @@ -35,7 +35,7 @@ public boolean requiresFeatures(){ @Override void initializeFeatures() { features = new FeatureManager(this, FeatureDataSource.DEFAULT_QUERY_LOOKAHEAD_BASES, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); initializeDrivingFeatures(); } diff --git a/src/main/java/org/broadinstitute/hellbender/engine/IntervalWalker.java b/src/main/java/org/broadinstitute/hellbender/engine/IntervalWalker.java index b9e5b764947..3bc3c4ce894 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/IntervalWalker.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/IntervalWalker.java @@ -36,7 +36,7 @@ void initializeFeatures() { // with ReadWalkers, typically), but with IntervalWalkers our query intervals are guaranteed // to be non-overlapping, since our interval parsing code always merges overlapping intervals. features = new FeatureManager(this, 0, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); if ( features.isEmpty() ) { // No available sources of Features for this tool features = null; } diff --git a/src/main/java/org/broadinstitute/hellbender/engine/ReadWalker.java b/src/main/java/org/broadinstitute/hellbender/engine/ReadWalker.java index 033ff6b0a91..0fd8a3cc1c1 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/ReadWalker.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/ReadWalker.java @@ -65,7 +65,7 @@ void setReadTraversalBounds() { void initializeFeatures() { //We override this method to change lookahead of the cache features = new FeatureManager(this, FEATURE_CACHE_LOOKAHEAD, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); if ( features.isEmpty() ) { // No available sources of Features discovered for this tool features = null; } diff --git a/src/main/java/org/broadinstitute/hellbender/engine/VariantWalkerBase.java b/src/main/java/org/broadinstitute/hellbender/engine/VariantWalkerBase.java index 301ad0dbf51..08ae9ade223 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/VariantWalkerBase.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/VariantWalkerBase.java @@ -63,7 +63,7 @@ void initializeFeatures() { // TODO: FeatureDataSource.DEFAULT_QUERY_LOOKAHEAD_BASES will likely hurt performance for tools like VQSR, // TODO: but let's test it features = new FeatureManager(this, DEFAULT_DRIVING_VARIANTS_LOOKAHEAD_BASES, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); initializeDrivingVariants(); } diff --git a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/AnnotateIntervals.java b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/AnnotateIntervals.java index 5eb25739fa8..18863b9398d 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/copynumber/AnnotateIntervals.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/copynumber/AnnotateIntervals.java @@ -169,7 +169,7 @@ public void onTraversalStart() { featureQueryLookahead, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); // always perform GC-content annotation logger.info("Adding GC-content annotator..."); diff --git a/src/test/java/org/broadinstitute/hellbender/engine/DummyPlaceholderGatkTool.java b/src/test/java/org/broadinstitute/hellbender/engine/DummyPlaceholderGatkTool.java index adc2b0d01e3..6b0ad3d8f36 100644 --- a/src/test/java/org/broadinstitute/hellbender/engine/DummyPlaceholderGatkTool.java +++ b/src/test/java/org/broadinstitute/hellbender/engine/DummyPlaceholderGatkTool.java @@ -29,6 +29,6 @@ public void traverse() { @Override void initializeFeatures(){ features = new FeatureManager(this, FeatureDataSource.DEFAULT_QUERY_LOOKAHEAD_BASES, cloudPrefetchBuffer, cloudIndexPrefetchBuffer, - referenceArguments.getReferencePath()); + getGenomicsDBOptions()); } } diff --git a/src/test/java/org/broadinstitute/hellbender/utils/test/FuncotatorTestUtils.java b/src/test/java/org/broadinstitute/hellbender/utils/test/FuncotatorTestUtils.java index 918656a2029..39551b2528f 100644 --- a/src/test/java/org/broadinstitute/hellbender/utils/test/FuncotatorTestUtils.java +++ b/src/test/java/org/broadinstitute/hellbender/utils/test/FuncotatorTestUtils.java @@ -54,9 +54,9 @@ private FuncotatorTestUtils() {} * @param interval genomic interval for the result. Typically, this would be the interval of the variant. Never {@link null}. * @param featureQueryLookahead When querying FeatureDataSources, cache this many extra bases of context beyond * the end of query intervals in anticipation of future queries. Must be >= 0. If uncertain, use zero. - * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use zero. - * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use zero. - * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, Path)} If uncertain, use {@code null}. + * @param cloudPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use zero. + * @param cloudIndexPrefetchBuffer See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use zero. + * @param reference See {@link FeatureManager#FeatureManager(CommandLineProgram, int, int, int, org.broadinstitute.hellbender.tools.genomicsdb.GenomicsDBOptions)} If uncertain, use {@code null}. * @return a {@link FeatureContext} ready for querying the funcotation factories on the given interval. Never {@code null}. */ @VisibleForTesting From a9e6044cfeecc2782a466f455b05321c3d94b425 Mon Sep 17 00:00:00 2001 From: Laura Gauthier Date: Mon, 5 Aug 2019 16:24:48 -0400 Subject: [PATCH 5/6] Address review comments --- .../tools/walkers/annotator/AnnotationUtils.java | 5 +---- .../allelespecific/AlleleSpecificAnnotation.java | 9 +++++++++ .../AssemblyBasedCallerArgumentCollection.java | 4 ++-- .../walkers/haplotypecaller/HaplotypeCallerEngine.java | 6 ------ .../haplotypecaller/HaplotypeCallerIntegrationTest.java | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) create mode 100644 src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AlleleSpecificAnnotation.java diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java index b167173bd93..d39537e0e98 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/AnnotationUtils.java @@ -50,10 +50,7 @@ public static String encodeAnyASList( final List somethingList) { * @return true if the annotation is expected to have values per-allele */ public static boolean isAlleleSpecific(final InfoFieldAnnotation annotation) { - if (annotation instanceof AlleleSpecificAnnotation) { - return true; - } - return false; + return annotation instanceof AlleleSpecificAnnotation; } /** diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AlleleSpecificAnnotation.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AlleleSpecificAnnotation.java new file mode 100644 index 00000000000..966f3476643 --- /dev/null +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/annotator/allelespecific/AlleleSpecificAnnotation.java @@ -0,0 +1,9 @@ +package org.broadinstitute.hellbender.tools.walkers.annotator.allelespecific; + +import org.broadinstitute.hellbender.tools.walkers.annotator.Annotation; + +/** + * This is a marker interface used to indicate which annotations are allele-specific. + */ +public interface AlleleSpecificAnnotation extends Annotation { +} diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java index dae4d5392fd..14ac333d8de 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerArgumentCollection.java @@ -110,10 +110,10 @@ public ReadThreadingAssembler createReadThreadingAssembler() { /** * The reference confidence mode makes it possible to emit a per-bp or summarized confidence estimate for a site being strictly homozygous-reference. * See https://software.broadinstitute.org/gatk/documentation/article.php?id=4017 for information about GVCFs. - * This is a BETA FEATURE for Mutect2 similar to the HaplotypeCaller reference confidence/GVCF mode. + * For Mutect2, this is a BETA feature that functions similarly to the HaplotypeCaller reference confidence/GVCF mode. */ @Advanced - @Argument(fullName=EMIT_REFERENCE_CONFIDENCE_LONG_NAME, shortName="ERC", doc="Mode for emitting reference confidence scores (BETA for Mutect2)", optional = true) + @Argument(fullName=EMIT_REFERENCE_CONFIDENCE_LONG_NAME, shortName="ERC", doc="Mode for emitting reference confidence scores (For Mutect2, this is a BETA feature)", optional = true) public ReferenceConfidenceMode emitReferenceConfidence = ReferenceConfidenceMode.NONE; protected abstract int getDefaultMaxMnpDistance(); diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java index 161453c0962..ffc22168928 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerEngine.java @@ -279,12 +279,6 @@ private void validateAndInitializeArgs() { " so reference confidence mode (" + AssemblyBasedCallerArgumentCollection.EMIT_REFERENCE_CONFIDENCE_LONG_NAME + ") must be specified."); } - - if (hcArgs.floorBlocks && !emitReferenceConfidence()) { - throw new UserException(HaplotypeCallerArgumentCollection.OUTPUT_BLOCK_LOWER_BOUNDS + " refers to GVCF blocks," + - " so reference confidence mode (" + AssemblyBasedCallerArgumentCollection.EMIT_REFERENCE_CONFIDENCE_LONG_NAME + - ") must be specified."); - } } private void initializeSamples() { diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java index 93ba3cd9609..f91ed398b57 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java @@ -373,7 +373,7 @@ else if (!vc.getAlternateAllele(0).equals(Allele.NON_REF_ALLELE)){ //there } /* - * Test that in VCF mode we're consistent with past GATK4 results + * Test that GQs are correct when the --floor-blocks argument is supplied */ @Test(dataProvider="HaplotypeCallerTestInputs") public void testFloorGVCFBlocks(final String inputFileName, final String referenceFileName) throws Exception { @@ -385,7 +385,7 @@ public void testFloorGVCFBlocks(final String inputFileName, final String referen final ArgumentsBuilder args = new ArgumentsBuilder().addInput(new File(inputFileName)) .addReference(new File(referenceFileName)) - .addInterval(new SimpleInterval("20:10000000-10100000")) + .addInterval(new SimpleInterval("20:10009880-10012631")) .addBooleanArgument(StandardArgumentDefinitions.ADD_OUTPUT_VCF_COMMANDLINE, false) .addArgument("pairHMM", "AVX_LOGLESS_CACHING") .addArgument("floor-blocks") From 135265e7ca3e011ef1133cb9f9b9f02bdfd51c08 Mon Sep 17 00:00:00 2001 From: Laura Gauthier Date: Tue, 6 Aug 2019 08:58:35 -0400 Subject: [PATCH 6/6] Warnings, again --- .../walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java index f91ed398b57..5293f13548e 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java @@ -394,7 +394,7 @@ public void testFloorGVCFBlocks(final String inputFileName, final String referen requestedGqBands.forEach(x -> args.addArgument("GQB",x)); runCommandLine(args); - final List allGqBands = new ArrayList(requestedGqBands); + final List allGqBands = new ArrayList<>(requestedGqBands); allGqBands.add("99"); allGqBands.add("0");