From 1c6dc217ee3f8a14c060426209cdf97479bb59f3 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 23 Feb 2022 16:37:00 -0500 Subject: [PATCH] starting to respond to review comments" --- .../hellbender/engine/AssemblyRegionIterator.java | 5 +++-- .../PileupDetectionArgumentCollection.java | 9 --------- .../AssemblyBasedCallerUtilsUnitTest.java | 14 +++++++++----- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/engine/AssemblyRegionIterator.java b/src/main/java/org/broadinstitute/hellbender/engine/AssemblyRegionIterator.java index b347ce7de03..196ee1203e8 100644 --- a/src/main/java/org/broadinstitute/hellbender/engine/AssemblyRegionIterator.java +++ b/src/main/java/org/broadinstitute/hellbender/engine/AssemblyRegionIterator.java @@ -88,7 +88,7 @@ public AssemblyRegionIterator(final MultiIntervalShard readShard, this.readCachingIterator = new ReadCachingIterator(readShard.iterator()); this.readCache = new ArrayDeque<>(); this.activityProfile = new BandPassActivityProfile(assemblyRegionArgs.maxProbPropagationDistance, assemblyRegionArgs.activeProbThreshold, BandPassActivityProfile.MAX_FILTER_SIZE, BandPassActivityProfile.DEFAULT_SIGMA, readHeader); - this.pendingAlignmentData = trackPileups? new ArrayDeque<>(): null; + this.pendingAlignmentData = trackPileups ? new ArrayDeque<>() : null; // We wrap our LocusIteratorByState inside an IntervalAlignmentContextIterator so that we get empty loci // for uncovered locations. This is critical for reproducing GATK 3.x behavior! @@ -218,7 +218,8 @@ private void fillNextAssemblyRegionWithReads( final AssemblyRegion region ) { } private void fillNextAssemblyRegionWithPileupData(final AssemblyRegion region){ - if (pendingAlignmentData==null){ + // Save ourselves the memory footprint and work of saving the pileups in the event they aren't needed for processing. + if (pendingAlignmentData == null){ return; } final List overlappingAlignmentData = new ArrayList<>(); diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/PileupDetectionArgumentCollection.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/PileupDetectionArgumentCollection.java index 4f2fcf27306..89ab0907cdf 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/PileupDetectionArgumentCollection.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/PileupDetectionArgumentCollection.java @@ -19,7 +19,6 @@ public final class PileupDetectionArgumentCollection { //TODO we currently don't see the same threshold from active region determination... public static final String PILEUP_DETECTION_ACETIVE_REGION_LOD_THRESHOLD_LONG_NAME = "pileup-detection-active-region-lod-threshold"; - // Arguments related to DRAGEN heuristics related to "read badness" intended to filter out false positives from the pileup detection code public static final String PILEUP_DETECTION_BAD_READ_RATIO_LONG_NAME = "pileup-detection-bad-read-tolerance"; public static final String PILEUP_DETECTION_PROPER_PAIR_READ_BADNESS_LONG_NAME = "pileup-detection-proper-pair-read-badness"; @@ -57,10 +56,6 @@ public final class PileupDetectionArgumentCollection { @Argument(fullName= "artifical-haplotype-filtering-kmer-size", doc = "Pileup Detection: Controls what size to kmerize reads to in order to select best supported artificial haplotypes", optional = true) public int filteringKmerSize = 10; - - - - /** * Percentage of reads required to support the alt for a variant to be considered */ @@ -71,7 +66,6 @@ public final class PileupDetectionArgumentCollection { @Argument(fullName= PILEUP_DETECTION_INDEL_THRESHOLD, doc = "Pileup Detection: Percentage of alt supporting reads in order to consider alt indel", optional = true) public double indelThreshold = 0.5; - @Hidden @Argument(fullName= PILEUP_DETECTION_ABSOLUTE_ALT_DEPTH, doc = "Pileup Detection: Absolute number of alt reads necessary to be included in pileup events", optional = true) public double pileupAbsoluteDepth = 0; @@ -79,9 +73,6 @@ public final class PileupDetectionArgumentCollection { @Argument(fullName= PILEUP_DETECTION_INDEL_SNP_BLOCKING_RANGE, doc = "Pileup Detection: Filters out pileup snps within this many bases of an assembled indel", optional = true) public int snpAdajacentToAssemblyIndel = 5; - - - /** * Arguments related to the "bad read filtering" where alleles that are supported primarily by reads that fail at least one of a number of heuristics will be filtered out */ diff --git a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtilsUnitTest.java b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtilsUnitTest.java index e8237cb8469..9baef8d02d5 100644 --- a/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtilsUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtilsUnitTest.java @@ -1391,7 +1391,8 @@ public Object[][] filterPileupHaplotypesDataProvider() { final Haplotype hapD = new Haplotype("ACCTGAA".getBytes()); final Haplotype hapF = new Haplotype("GAAGAAG".getBytes()); // testing repeated kmers - Map flatSupportAllKmers = new HashMap() { private static final long serialVersionUID = 0L; { + Map flatSupportAllKmers = new HashMap() { + private static final long serialVersionUID = 0L; { put(new Kmer("ACC"), 1); put(new Kmer("CCT"), 1); put(new Kmer("CTG"), 1); @@ -1405,7 +1406,8 @@ public Object[][] filterPileupHaplotypesDataProvider() { put(new Kmer("AAG"), 1); }}; - Map hapDKmersHighSupport = new HashMap() { private static final long serialVersionUID = 0L;{ + Map hapDKmersHighSupport = new HashMap() { + private static final long serialVersionUID = 0L;{ put(new Kmer("ACC"), 10); put(new Kmer("CCT"), 10); put(new Kmer("CTG"), 10); @@ -1419,7 +1421,8 @@ public Object[][] filterPileupHaplotypesDataProvider() { put(new Kmer("AAG"), 1); }}; - Map hapDKmers = new HashMap() { private static final long serialVersionUID = 0L; { + Map hapDKmers = new HashMap() { + private static final long serialVersionUID = 0L; { put(new Kmer("ACC"), 10); put(new Kmer("CCT"), 10); put(new Kmer("CTG"), 10); @@ -1427,7 +1430,8 @@ public Object[][] filterPileupHaplotypesDataProvider() { put(new Kmer("GAA"), 10); }}; - Map hapFRepeatedKmers = new HashMap() { private static final long serialVersionUID = 0L; { + Map hapFRepeatedKmers = new HashMap() { + private static final long serialVersionUID = 0L; { put(new Kmer("GAA"), 1); }}; @@ -1439,7 +1443,7 @@ public Object[][] filterPileupHaplotypesDataProvider() { new Object[]{Arrays.asList(hapA,hapB,hapC,hapD),flatSupportAllKmers,2,3,Arrays.asList(hapA,hapB)}, new Object[]{Arrays.asList(hapA,hapB,hapC,hapD),flatSupportAllKmers,3,3,Arrays.asList(hapA,hapB,hapC)}, - // Repetative kmers in hapF don't get double counted + // Repetitive kmers in hapF don't get double counted new Object[]{Arrays.asList(hapA,hapB,hapD,hapF),hapFRepeatedKmers,2,3,Arrays.asList(hapF,hapD)}, new Object[]{Arrays.asList(hapA,hapB,hapD,hapF),hapFRepeatedKmers,1,3,Arrays.asList(hapD)}, //currently repeated kmers only count as singular evidence