From b194fab05a2ad5d43e2539ce6b77366025b7381e Mon Sep 17 00:00:00 2001 From: James Date: Mon, 28 Jan 2019 15:50:24 -0500 Subject: [PATCH 1/4] replaced some iterators with indexed-for-lists in hopes of making an impact on the world --- .../ReferenceConfidenceModel.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java index 50b68f388d2..2bf2a795f56 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java @@ -193,18 +193,18 @@ public List calculateRefConfidence(final Haplotype refHaplotype, final String sampleName = readLikelihoods.getSample(0); final int globalRefOffset = refSpan.getStart() - activeRegion.getExtendedSpan().getStart(); - for ( final ReadPileup pileup : refPileups ) { + for (int i = 0, size = refPileups.size(); i < size; i++) { + ReadPileup pileup = refPileups.get(i); final Locatable curPos = pileup.getLocation(); final int offset = curPos.getStart() - refSpan.getStart(); final VariantContext overlappingSite = GATKVariantContextUtils.getOverlappingVariantContext(curPos, variantCalls); final List currentPriors = getMatchingPriors(curPos, overlappingSite, VCpriors); - if ( overlappingSite != null && overlappingSite.getStart() == curPos.getStart() ) { + if (overlappingSite != null && overlappingSite.getStart() == curPos.getStart()) { if (applyPriors) { results.add(PosteriorProbabilitiesUtils.calculatePosteriorProbs(overlappingSite, currentPriors, numRefSamplesForPrior, options)); - } - else { + } else { results.add(overlappingSite); } } else { @@ -420,9 +420,15 @@ protected static boolean isAltAfterAssembly(final PileupElement element, final b * @param priorList priors within the current ActiveRegion * @return prior VCs representing the same variant position as call */ - List getMatchingPriors(final Locatable curPos, final VariantContext call, final List priorList) { + private List getMatchingPriors(final Locatable curPos, final VariantContext call, final List priorList) { final int position = call != null ? call.getStart() : curPos.getStart(); - return priorList.stream().filter(vc -> position == vc.getStart()).collect(Collectors.toList()); + List list = new ArrayList<>(priorList.size()); + for (int i = 0, size = priorList.size(); i < size; i++) { + if (position == priorList.get(i).getStart()) { + list.add(priorList.get(i)); + } + } + return list; } /** From 4b8941b9f8bf0ed150d2b39626a98d852c5eda0f Mon Sep 17 00:00:00 2001 From: James Date: Thu, 7 Feb 2019 16:10:37 -0500 Subject: [PATCH 2/4] responded to comments --- .../ReferenceConfidenceModel.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java index 2bf2a795f56..8cc29ed0ac1 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java @@ -193,13 +193,14 @@ public List calculateRefConfidence(final Haplotype refHaplotype, final String sampleName = readLikelihoods.getSample(0); final int globalRefOffset = refSpan.getStart() - activeRegion.getExtendedSpan().getStart(); - for (int i = 0, size = refPileups.size(); i < size; i++) { - ReadPileup pileup = refPileups.get(i); + final int refPileupsSize = refPileups.size(); + for (int i = 0; i < refPileupsSize; i++) { + final ReadPileup pileup = refPileups.get(i); final Locatable curPos = pileup.getLocation(); final int offset = curPos.getStart() - refSpan.getStart(); final VariantContext overlappingSite = GATKVariantContextUtils.getOverlappingVariantContext(curPos, variantCalls); - final List currentPriors = getMatchingPriors(curPos, overlappingSite, VCpriors); + final List currentPriors = VCpriors.isEmpty() ? Collections.emptyList() : getMatchingPriors(curPos, overlappingSite, VCpriors); if (overlappingSite != null && overlappingSite.getStart() == curPos.getStart()) { if (applyPriors) { results.add(PosteriorProbabilitiesUtils.calculatePosteriorProbs(overlappingSite, currentPriors, @@ -422,13 +423,15 @@ protected static boolean isAltAfterAssembly(final PileupElement element, final b */ private List getMatchingPriors(final Locatable curPos, final VariantContext call, final List priorList) { final int position = call != null ? call.getStart() : curPos.getStart(); - List list = new ArrayList<>(priorList.size()); - for (int i = 0, size = priorList.size(); i < size; i++) { + final List matchedPriors = new ArrayList<>(priorList.size()); + // NOTE: a for loop is used here because this method ends up being called per-pileup, per-read and thus using faster alternates saves runtime + final int priorsListSize = priorList.size() + for (int i = 0; i < priorsListSize; i++) { if (position == priorList.get(i).getStart()) { - list.add(priorList.get(i)); + matchedPriors.add(priorList.get(i)); } } - return list; + return matchedPriors; } /** From 7881211d864102c8560ea4fd6aed747fb09c50c3 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 8 Feb 2019 15:04:15 -0500 Subject: [PATCH 3/4] fixing the error --- .../tools/walkers/haplotypecaller/ReferenceConfidenceModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java index 8cc29ed0ac1..2130ee6241e 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java @@ -425,7 +425,7 @@ private List getMatchingPriors(final Locatable curPos, final Var final int position = call != null ? call.getStart() : curPos.getStart(); final List matchedPriors = new ArrayList<>(priorList.size()); // NOTE: a for loop is used here because this method ends up being called per-pileup, per-read and thus using faster alternates saves runtime - final int priorsListSize = priorList.size() + final int priorsListSize = priorList.size(); for (int i = 0; i < priorsListSize; i++) { if (position == priorList.get(i).getStart()) { matchedPriors.add(priorList.get(i)); From 73d6f78ea55c34c41730f1a722bc7fac993b33d9 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 8 Feb 2019 15:32:51 -0500 Subject: [PATCH 4/4] responding to final comments --- .../walkers/haplotypecaller/ReferenceConfidenceModel.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java index 2130ee6241e..efb50b0fb82 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/ReferenceConfidenceModel.java @@ -193,6 +193,7 @@ public List calculateRefConfidence(final Haplotype refHaplotype, final String sampleName = readLikelihoods.getSample(0); final int globalRefOffset = refSpan.getStart() - activeRegion.getExtendedSpan().getStart(); + // Note, we use an indexed for-loop here because this method has a large impact on the profile of HaplotypeCaller runtime in GVCF mode final int refPileupsSize = refPileups.size(); for (int i = 0; i < refPileupsSize; i++) { final ReadPileup pileup = refPileups.get(i); @@ -424,7 +425,7 @@ protected static boolean isAltAfterAssembly(final PileupElement element, final b private List getMatchingPriors(final Locatable curPos, final VariantContext call, final List priorList) { final int position = call != null ? call.getStart() : curPos.getStart(); final List matchedPriors = new ArrayList<>(priorList.size()); - // NOTE: a for loop is used here because this method ends up being called per-pileup, per-read and thus using faster alternates saves runtime + // NOTE: a for loop is used here because this method ends up being called per-pileup, per-read and using a loop instead of streaming saves runtime final int priorsListSize = priorList.size(); for (int i = 0; i < priorsListSize; i++) { if (position == priorList.get(i).getStart()) {