From 464a704f66f1609d7aee1816e2a593e39bdef99e Mon Sep 17 00:00:00 2001 From: kvinter Date: Wed, 15 Aug 2018 21:59:55 -0400 Subject: [PATCH 1/9] Compressed subclass (SAMFileDestination) into HaplotypeBAMDestination. --- .../haplotype/HaplotypeBAMDestination.java | 47 +++++++++++++++---- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java index 5cb4201a7af..354ef70238f 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java @@ -3,18 +3,24 @@ import htsjdk.samtools.SAMFileHeader; import htsjdk.samtools.SAMProgramRecord; import htsjdk.samtools.SAMReadGroupRecord; +import java.nio.file.Path; +import org.broadinstitute.hellbender.utils.read.SAMFileGATKReadWriter; + import org.broadinstitute.hellbender.utils.Utils; import org.broadinstitute.hellbender.utils.read.GATKRead; +import org.broadinstitute.hellbender.utils.read.ReadUtils; import java.util.ArrayList; import java.util.List; /** - * Utility class that allows easy creation of destinations for the HaplotypeBAMWriters + * Class used to direct output from a HaplotypeBAMWriter to a bam/sam file. * */ -public abstract class HaplotypeBAMDestination { + +public final class HaplotypeBAMDestination { + private final SAMFileGATKReadWriter samWriter; private final SAMFileHeader bamOutputHeader; private final String haplotypeReadGroupID; private final static String haplotypeSampleTag = "HC"; @@ -22,10 +28,19 @@ public abstract class HaplotypeBAMDestination { /** * Create a new HaplotypeBAMDestination * - * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination. - * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads + * @param outPath path where output is written (doesn't have to be local) + * @param createBamOutIndex true to create an index file for the bamout + * @param createBamOutMD5 true to create an md5 file for the bamout + * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination, must not be null + * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads */ - protected HaplotypeBAMDestination(SAMFileHeader sourceHeader, final String haplotypeReadGroupID) { + protected HaplotypeBAMDestination( + final Path outPath, + final boolean createBamOutIndex, + final boolean createBamOutMD5, + final SAMFileHeader sourceHeader, + final String haplotypeReadGroupID) + { Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); Utils.nonNull(haplotypeReadGroupID, "haplotypeReadGroupID cannot be null"); this.haplotypeReadGroupID = haplotypeReadGroupID; @@ -45,6 +60,15 @@ protected HaplotypeBAMDestination(SAMFileHeader sourceHeader, final String haplo bamOutputHeader.setReadGroups(readGroups); bamOutputHeader.addProgramRecord(new SAMProgramRecord("HalpotypeBAMWriter")); + + samWriter = new SAMFileGATKReadWriter(ReadUtils.createCommonSAMWriter( + outPath, + null, + getBAMOutputHeader(), // use the header derived from the source header by HaplotypeBAMDestination + false, + createBamOutIndex, + createBamOutMD5 + )); } /** @@ -52,7 +76,10 @@ protected HaplotypeBAMDestination(SAMFileHeader sourceHeader, final String haplo * * @param read the read to write out */ - public abstract void add(final GATKRead read); + public void add(final GATKRead read){ + Utils.nonNull(read, "read cannot be null"); + samWriter.addRead(read); + }; /** * Get the read group ID that is used by this writer when writing halpotypes as reads. @@ -69,10 +96,12 @@ protected HaplotypeBAMDestination(SAMFileHeader sourceHeader, final String haplo public String getHaplotypeSampleTag() { return haplotypeSampleTag; } /** - * Close the destination - * + * Close any resources associated with this destination. */ - abstract void close(); + + void close(){ + samWriter.close(); + }; /** * Get the SAMFileHeader that is used for writing the output for this destination. From 51315b7915187f5f2668e016b2f9559724c83ead Mon Sep 17 00:00:00 2001 From: kvinter Date: Thu, 16 Aug 2018 16:03:35 -0400 Subject: [PATCH 2/9] Work in progress --- .../haplotype/CalledHaplotypeBAMWriter.java | 7 +++- .../haplotype/HaplotypeBAMDestination.java | 4 +- .../utils/haplotype/HaplotypeBAMWriter.java | 41 +++++++------------ .../utils/haplotype/SAMFileDestination.java | 2 +- .../haplotype/HaplotypeBAMWriterUnitTest.java | 5 +-- 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java index 5183a412140..ca00cfea3e2 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java @@ -7,6 +7,7 @@ import org.broadinstitute.hellbender.utils.Utils; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.Set; @@ -44,11 +45,13 @@ public void writeReadsAlignedToHaplotypes(final Collection haplotypes Utils.nonNull(calledHaplotypes, "calledHaplotypes cannot be null"); Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); - if (calledHaplotypes.isEmpty()) { // only write out called haplotypes + if (calledHaplotypes.isEmpty() && writerType.equals("CALLED_HAPLOTYPES")) { // only write out called haplotypes return; } - writeHaplotypesAsReads(calledHaplotypes, calledHaplotypes, paddedReferenceLoc); + Collection haplotypesToWrite = writerType.equals("CALLED_HAPLOTYPES") ? calledHaplotypes : haplotypes; + Set bestHaplotypesToWrite = writerType.equals("CALLED_HAPLOTYPES") ? calledHaplotypes : new LinkedHashSet<>(bestHaplotypes); + writeHaplotypesAsReads(haplotypesToWrite, bestHaplotypesToWrite, paddedReferenceLoc); final int sampleCount = readLikelihoods.numberOfSamples(); for (int i = 0; i < sampleCount; i++) { diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java index 354ef70238f..9449d3bdace 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java @@ -19,7 +19,7 @@ * */ -public final class HaplotypeBAMDestination { +public class HaplotypeBAMDestination { private final SAMFileGATKReadWriter samWriter; private final SAMFileHeader bamOutputHeader; private final String haplotypeReadGroupID; @@ -41,6 +41,8 @@ protected HaplotypeBAMDestination( final SAMFileHeader sourceHeader, final String haplotypeReadGroupID) { + Utils.nonNull(outPath, "outputPath cannot be null"); + Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); Utils.nonNull(haplotypeReadGroupID, "haplotypeReadGroupID cannot be null"); this.haplotypeReadGroupID = haplotypeReadGroupID; diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index 5fab38761b8..494d8e5632b 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -11,7 +11,6 @@ import org.broadinstitute.hellbender.utils.Utils; import org.broadinstitute.hellbender.utils.genotyper.ReadLikelihoods; -import java.io.File; import java.util.Collection; import java.util.function.Function; import java.util.Set; @@ -33,6 +32,7 @@ public abstract class HaplotypeBAMWriter implements AutoCloseable { private static final int otherMQ = 0; protected final HaplotypeBAMDestination output; + private WriterType writerType; private boolean writeHaplotypes = true; /** @@ -43,26 +43,14 @@ public enum WriterType { * A mode that's for method developers. Writes out all of the possible * haplotypes considered, as well as reads aligned to each */ - ALL_POSSIBLE_HAPLOTYPES(AllHaplotypeBAMWriter::new), + ALL_POSSIBLE_HAPLOTYPES, /** * A mode for users. Writes out the reads aligned only to the called * haplotypes. Useful to understand why the caller is calling what it is */ - CALLED_HAPLOTYPES(CalledHaplotypeBAMWriter::new); + CALLED_HAPLOTYPES; - final private Function factory; - - WriterType(Function factory) { - this.factory = factory; - } - - /** - * Create an instance of the HaplotypeBAMWriter corresponding to this type. - */ - public HaplotypeBAMWriter create(HaplotypeBAMDestination destination) { - Utils.nonNull(destination, "destination cannot be null"); - return factory.apply(destination); } } /** @@ -75,17 +63,13 @@ public HaplotypeBAMWriter create(HaplotypeBAMDestination destination) { * @param sourceHeader the header of the input BAMs used to make calls, must not be null. * @return a new HaplotypeBAMWriter */ - public static HaplotypeBAMWriter create( + public HaplotypeBAMWriter( final WriterType type, final Path outputPath, final boolean createBamOutIndex, final boolean createBamOutMD5, final SAMFileHeader sourceHeader) { - Utils.nonNull(type, "type cannot be null"); - Utils.nonNull(outputPath, "outputPath cannot be null"); - Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); - - return create(type, new SAMFileDestination(outputPath, createBamOutIndex, createBamOutMD5, sourceHeader, DEFAULT_HAPLOTYPE_READ_GROUP_ID)); + this(type, new HaplotypeBAMDestination(outputPath, createBamOutIndex, createBamOutMD5, sourceHeader, DEFAULT_HAPLOTYPE_READ_GROUP_ID)); } /** @@ -97,11 +81,13 @@ public static HaplotypeBAMWriter create( * * @return a new HaplotypeBAMWriter */ - public static HaplotypeBAMWriter create(final WriterType type, final HaplotypeBAMDestination destination) { + public HaplotypeBAMWriter( + final WriterType type, + final HaplotypeBAMDestination destination) { Utils.nonNull(type, "type cannot be null"); Utils.nonNull(destination, "destination cannot be null"); - - return type.create(destination); + this.writerType = type; + this.output = destination; } /** @@ -111,6 +97,7 @@ public static HaplotypeBAMWriter create(final WriterType type, final HaplotypeBA * have its presorted bit set to false, as reads may come in out of order during writing. */ protected HaplotypeBAMWriter(final HaplotypeBAMDestination output) { + Utils.nonNull(output, "output cannot be null"); this.output = output; } @@ -132,11 +119,13 @@ public void close() { * @param calledHaplotypes a list of the haplotypes that where actually called as non-reference * @param readLikelihoods a map from sample -> likelihoods for each read for each of the best haplotypes */ - public abstract void writeReadsAlignedToHaplotypes(final Collection haplotypes, + public void writeReadsAlignedToHaplotypes(final Collection haplotypes, final Locatable paddedReferenceLoc, final Collection bestHaplotypes, final Set calledHaplotypes, - final ReadLikelihoods readLikelihoods); + final ReadLikelihoods readLikelihoods){ + this.writerType; + } /** * Write out read aligned to haplotype to the BAM file diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java index f4f5f69813f..c3de2d6357b 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java @@ -31,7 +31,7 @@ public SAMFileDestination( final SAMFileHeader sourceHeader, final String haplotypeReadGroupID) { - super(sourceHeader, haplotypeReadGroupID); + super(outPath, createBamOutIndex, createBamOutMD5,sourceHeader, haplotypeReadGroupID); samWriter = new SAMFileGATKReadWriter(ReadUtils.createCommonSAMWriter( outPath, null, diff --git a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java index 613612a2e21..b157a8871f8 100644 --- a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java @@ -133,7 +133,7 @@ public Object[][] makeReadsLikelikhoodData() { ) throws IOException { final Path outPath = GATKBaseTest.createTempFile("haplotypeBamWriterTest", outputFileExtension).toPath(); - final HaplotypeBAMDestination fileDest = new SAMFileDestination(outPath, createIndex, createMD5, samHeader, "TestHaplotypeRG"); + final HaplotypeBAMDestination fileDest = new HaplotypeBAMDestination(outPath, createIndex, createMD5, samHeader, "TestHaplotypeRG"); try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, fileDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( @@ -287,12 +287,11 @@ private Haplotype makeHaplotype(final String bases) { private class MockValidatingDestination extends HaplotypeBAMDestination { private final String expectedBaseSignature; // bases expected for the synthesized haplotype read - public int readCount = 0; // number of reads written to this destination public boolean foundBases = false; // true we've seen a read that contains the expectedBaseSignature private MockValidatingDestination(String baseSignature) { - super(samHeader, "testGroupID"); + super(GATKBaseTest.createTempFile("haplotypeBamWriterTest", ".bam").toPath(), false, false, samHeader, "testGroupID"); expectedBaseSignature = baseSignature; } From 5cbbce381b2ecf64a135ece14256be2fdf8ac664 Mon Sep 17 00:00:00 2001 From: kvinter Date: Fri, 17 Aug 2018 15:38:58 -0400 Subject: [PATCH 3/9] Attempt to fix #944. --- .../AssemblyBasedCallerUtils.java | 2 +- .../haplotype/AllHaplotypeBAMWriter.java | 47 -------------- .../haplotype/CalledHaplotypeBAMWriter.java | 63 ------------------- .../utils/haplotype/HaplotypeBAMWriter.java | 48 ++++++++------ .../utils/haplotype/SAMFileDestination.java | 61 ------------------ .../haplotype/HaplotypeBAMWriterUnitTest.java | 36 ++++------- 6 files changed, 42 insertions(+), 215 deletions(-) delete mode 100644 src/main/java/org/broadinstitute/hellbender/utils/haplotype/AllHaplotypeBAMWriter.java delete mode 100644 src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java delete mode 100644 src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java diff --git a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtils.java b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtils.java index 86ba3276443..9b8402933c3 100644 --- a/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtils.java +++ b/src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/AssemblyBasedCallerUtils.java @@ -197,7 +197,7 @@ public static Optional createBamWriter(final AssemblyBasedCa final boolean createBamOutMD5, final SAMFileHeader header) { return args.bamOutputPath != null ? - Optional.of(HaplotypeBAMWriter.create(args.bamWriterType, IOUtils.getPath(args.bamOutputPath), createBamOutIndex, createBamOutMD5, header)) : + Optional.of(new HaplotypeBAMWriter(args.bamWriterType, IOUtils.getPath(args.bamOutputPath), createBamOutIndex, createBamOutMD5, header)) : Optional.empty(); } diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/AllHaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/AllHaplotypeBAMWriter.java deleted file mode 100644 index 094911edf4c..00000000000 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/AllHaplotypeBAMWriter.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.broadinstitute.hellbender.utils.haplotype; - -import htsjdk.samtools.util.Locatable; -import org.broadinstitute.hellbender.utils.genotyper.ReadLikelihoods; -import org.broadinstitute.hellbender.utils.read.GATKRead; -import org.broadinstitute.hellbender.utils.Utils; - -import java.util.Collection; -import java.util.LinkedHashSet; -import java.util.Set; - -/** - * A haplotype bam writer that writes out all haplotypes as reads and then - * the alignment of reach read to its best match among the best haplotypes. - * - * Primarily useful for people working on the HaplotypeCaller method itself - */ -final class AllHaplotypeBAMWriter extends HaplotypeBAMWriter { - - public AllHaplotypeBAMWriter(final HaplotypeBAMDestination destination) { - super(destination); - } - - /** - * {@inheritDoc} - */ - @Override - public void writeReadsAlignedToHaplotypes(final Collection haplotypes, - final Locatable paddedReferenceLoc, - final Collection bestHaplotypes, - final Set calledHaplotypes, - final ReadLikelihoods readLikelihoods) { - - Utils.nonNull(haplotypes, "haplotypes cannot be null"); - Utils.nonNull(paddedReferenceLoc, "paddedReferenceLoc cannot be null"); - Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); - - writeHaplotypesAsReads(haplotypes, new LinkedHashSet<>(bestHaplotypes), paddedReferenceLoc); - - final int sampleCount = readLikelihoods.numberOfSamples(); - for (int i = 0; i < sampleCount; i++) { - for (final GATKRead read : readLikelihoods.sampleReads(i)) { - writeReadAgainstHaplotype(read); - } - } - } -} diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java deleted file mode 100644 index ca00cfea3e2..00000000000 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/CalledHaplotypeBAMWriter.java +++ /dev/null @@ -1,63 +0,0 @@ -package org.broadinstitute.hellbender.utils.haplotype; - -import htsjdk.samtools.util.Locatable; - -import org.broadinstitute.hellbender.utils.genotyper.ReadLikelihoods; -import org.broadinstitute.hellbender.utils.read.GATKRead; -import org.broadinstitute.hellbender.utils.Utils; - -import java.util.Collection; -import java.util.LinkedHashSet; -import java.util.Set; - - -/** - * Writes a BAM containing just the reads in stratifiedReadMap aligned to their - * most likely haplotype among all of the called haplotypes. - * - * Primarily useful for users of the HaplotypeCaller who want to better understand the - * support of their calls w.r.t. the reads. - * - */ -final class CalledHaplotypeBAMWriter extends HaplotypeBAMWriter { - public CalledHaplotypeBAMWriter(final HaplotypeBAMDestination destination) { - super(destination); - } - - /** - * Write out a BAM representing for the haplotype caller at this site - * - * @param haplotypes a list of all possible haplotypes at this loc, must not be null - * @param paddedReferenceLoc the span of the based reference here, must not be null - * @param bestHaplotypes a list of the best (a subset of all) haplotypes that actually went forward into genotyping, - * must not be null - * @param calledHaplotypes a list of the haplotypes at where actually called as non-reference, must not be null - * @param readLikelihoods a map from sample -> likelihoods for each read for each of the best haplotypes - */ - @Override - public void writeReadsAlignedToHaplotypes(final Collection haplotypes, - final Locatable paddedReferenceLoc, - final Collection bestHaplotypes, - final Set calledHaplotypes, - final ReadLikelihoods readLikelihoods) { - Utils.nonNull(haplotypes, "haplotypes cannot be null"); - Utils.nonNull(paddedReferenceLoc, "paddedReferenceLoc cannot be null"); - Utils.nonNull(calledHaplotypes, "calledHaplotypes cannot be null"); - Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); - - if (calledHaplotypes.isEmpty() && writerType.equals("CALLED_HAPLOTYPES")) { // only write out called haplotypes - return; - } - - Collection haplotypesToWrite = writerType.equals("CALLED_HAPLOTYPES") ? calledHaplotypes : haplotypes; - Set bestHaplotypesToWrite = writerType.equals("CALLED_HAPLOTYPES") ? calledHaplotypes : new LinkedHashSet<>(bestHaplotypes); - writeHaplotypesAsReads(haplotypesToWrite, bestHaplotypesToWrite, paddedReferenceLoc); - - final int sampleCount = readLikelihoods.numberOfSamples(); - for (int i = 0; i < sampleCount; i++) { - for (final GATKRead read : readLikelihoods.sampleReads(i)) { - writeReadAgainstHaplotype(read); - } - } - } -} diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index 494d8e5632b..7c58a3b367f 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -12,6 +12,7 @@ import org.broadinstitute.hellbender.utils.genotyper.ReadLikelihoods; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.function.Function; import java.util.Set; @@ -19,7 +20,7 @@ * A BAMWriter that aligns reads to haplotypes and emits their best alignments to a destination. * */ -public abstract class HaplotypeBAMWriter implements AutoCloseable { +public class HaplotypeBAMWriter implements AutoCloseable { /** * Allows us to write out unique names for our synthetic haplotype reads */ @@ -69,6 +70,7 @@ public HaplotypeBAMWriter( final boolean createBamOutIndex, final boolean createBamOutMD5, final SAMFileHeader sourceHeader) { + this(type, new HaplotypeBAMDestination(outputPath, createBamOutIndex, createBamOutMD5, sourceHeader, DEFAULT_HAPLOTYPE_READ_GROUP_ID)); } @@ -84,24 +86,13 @@ public HaplotypeBAMWriter( public HaplotypeBAMWriter( final WriterType type, final HaplotypeBAMDestination destination) { + Utils.nonNull(type, "type cannot be null"); Utils.nonNull(destination, "destination cannot be null"); this.writerType = type; this.output = destination; } - /** - * Create a new HaplotypeBAMWriter writing its output to the given destination - * - * @param output the output destination, must not be null. Note that SAM writer associated with the destination must - * have its presorted bit set to false, as reads may come in out of order during writing. - */ - protected HaplotypeBAMWriter(final HaplotypeBAMDestination output) { - - Utils.nonNull(output, "output cannot be null"); - this.output = output; - } - /** * Close any output resource associated with this writer. */ @@ -112,6 +103,7 @@ public void close() { /** * Write out a BAM representing for the haplotype caller at this site + * writerType (ALL_POSSIBLE_HAPLOTYPES or CALLED_HAPLOTYPES) determines inputs to writeHaplotypesAsReads * * @param haplotypes a list of all possible haplotypes at this loc * @param paddedReferenceLoc the span of the based reference here @@ -120,11 +112,31 @@ public void close() { * @param readLikelihoods a map from sample -> likelihoods for each read for each of the best haplotypes */ public void writeReadsAlignedToHaplotypes(final Collection haplotypes, - final Locatable paddedReferenceLoc, - final Collection bestHaplotypes, - final Set calledHaplotypes, - final ReadLikelihoods readLikelihoods){ - this.writerType; + final Locatable paddedReferenceLoc, + final Collection bestHaplotypes, + final Set calledHaplotypes, + final ReadLikelihoods readLikelihoods) { + + Utils.nonNull(haplotypes, "haplotypes cannot be null"); + Utils.nonNull(paddedReferenceLoc, "paddedReferenceLoc cannot be null"); + Utils.nonNull(calledHaplotypes, "calledHaplotypes cannot be null"); + Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); + + if (calledHaplotypes.isEmpty() && writerType.equals(WriterType.CALLED_HAPLOTYPES)) { // only write out called haplotypes + return; + } + + Collection haplotypesToWrite = writerType.equals(WriterType.CALLED_HAPLOTYPES) ? calledHaplotypes : haplotypes; + Set bestHaplotypesToWrite = writerType.equals(WriterType.CALLED_HAPLOTYPES) ? calledHaplotypes : new LinkedHashSet<>(bestHaplotypes); + + writeHaplotypesAsReads(haplotypesToWrite, bestHaplotypesToWrite, paddedReferenceLoc); + + final int sampleCount = readLikelihoods.numberOfSamples(); + for (int i = 0; i < sampleCount; i++) { + for (final GATKRead read : readLikelihoods.sampleReads(i)) { + writeReadAgainstHaplotype(read); + } + } } /** diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java deleted file mode 100644 index c3de2d6357b..00000000000 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java +++ /dev/null @@ -1,61 +0,0 @@ -package org.broadinstitute.hellbender.utils.haplotype; - -import htsjdk.samtools.SAMFileHeader; -import java.nio.file.Path; -import org.broadinstitute.hellbender.utils.read.GATKRead; -import org.broadinstitute.hellbender.utils.Utils; -import org.broadinstitute.hellbender.utils.read.ReadUtils; -import org.broadinstitute.hellbender.utils.read.SAMFileGATKReadWriter; - -import java.io.File; - -/** - * Class used to direct output from a HaplotypeBAMWriter to a bam/sam file. - */ -public final class SAMFileDestination extends HaplotypeBAMDestination { - private final SAMFileGATKReadWriter samWriter; - - /** - * Create a new file destination. - * - * @param outPath path where output is written (doesn't have to be local) - * @param createBamOutIndex true to create an index file for the bamout - * @param createBamOutMD5 true to create an md5 file for the bamout - * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination, must not be null - * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads - */ - public SAMFileDestination( - final Path outPath, - final boolean createBamOutIndex, - final boolean createBamOutMD5, - final SAMFileHeader sourceHeader, - final String haplotypeReadGroupID) - { - super(outPath, createBamOutIndex, createBamOutMD5,sourceHeader, haplotypeReadGroupID); - samWriter = new SAMFileGATKReadWriter(ReadUtils.createCommonSAMWriter( - outPath, - null, - getBAMOutputHeader(), // use the header derived from the source header by HaplotypeBAMDestination - false, - createBamOutIndex, - createBamOutMD5 - )); - } - - /** - * Close any resources associated with this destination. - */ - @Override - void close() { samWriter.close(); } - - /** - * Write a read to the output file specified by this destination. - * - * @param read the read to write out, must not be null - */ - @Override - public void add(final GATKRead read) { - Utils.nonNull(read, "read cannot be null"); - samWriter.addRead(read); - } -} \ No newline at end of file diff --git a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java index b157a8871f8..24d3a8044a4 100644 --- a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java @@ -29,26 +29,13 @@ import java.io.File; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; public class HaplotypeBAMWriterUnitTest extends GATKBaseTest { private final SAMFileHeader samHeader = ArtificialReadUtils.createArtificialSamHeader(20, 1, 1000); private final String expectedFilePath = getToolTestDataDir() + "/expected/"; - @Test - public void testSimpleCreate() throws Exception { - final MockValidatingDestination writer = new MockValidatingDestination(null); - Assert.assertTrue(HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.CALLED_HAPLOTYPES, writer) instanceof CalledHaplotypeBAMWriter); - Assert.assertTrue(HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, writer) instanceof AllHaplotypeBAMWriter); - } - @DataProvider(name="ReadsLikelikhoodData") public Object[][] makeReadsLikelikhoodData() { final String haplotypeBaseSignature = "ACTGAAGGTTCC"; @@ -135,12 +122,12 @@ public Object[][] makeReadsLikelikhoodData() { final Path outPath = GATKBaseTest.createTempFile("haplotypeBamWriterTest", outputFileExtension).toPath(); final HaplotypeBAMDestination fileDest = new HaplotypeBAMDestination(outPath, createIndex, createMD5, samHeader, "TestHaplotypeRG"); - try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, fileDest)) { + try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, fileDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, haplotypes, - null, // called haplotypes + new HashSet<>(), // called haplotypes readLikelihoods); } @@ -165,12 +152,12 @@ public void testCreateSAMFromHeader( ) throws IOException { final Path outputPath = GATKBaseTest.createTempFile("fromHeaderSAM", ".sam").toPath(); - try (HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, outputPath, false, false, samHeader)) { + try (HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, outputPath, false, false, samHeader)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, haplotypes, - null, // called haplotypes + new HashSet<>(), // called haplotypes readLikelihoods); } @@ -188,13 +175,12 @@ public void testCreateSAMFromHeader( ) { final MockValidatingDestination mockDest = new MockValidatingDestination(haplotypeBaseSignature); - - try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, mockDest)) { + try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, mockDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, haplotypes, - null, // called haplotypes + new HashSet<>(), // called haplotypes readLikelihoods); } @@ -216,7 +202,7 @@ public void testCreateSAMFromHeader( Set calledHaplotypes = new LinkedHashSet<>(1); calledHaplotypes.addAll(haplotypes); - try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.CALLED_HAPLOTYPES, mockDest)) { + try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.CALLED_HAPLOTYPES, mockDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, @@ -240,7 +226,7 @@ public void testCreateSAMFromHeader( { final MockValidatingDestination mockDest = new MockValidatingDestination(haplotypeBaseSignature); - try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.CALLED_HAPLOTYPES, mockDest)) { + try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.CALLED_HAPLOTYPES, mockDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, @@ -263,14 +249,14 @@ public void testCreateSAMFromHeader( { final MockValidatingDestination mockDest = new MockValidatingDestination(haplotypeBaseSignature); - try (final HaplotypeBAMWriter haplotypeBAMWriter = HaplotypeBAMWriter.create(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, mockDest)) { + try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, mockDest)) { haplotypeBAMWriter.setWriteHaplotypes(false); haplotypeBAMWriter.writeReadsAlignedToHaplotypes( haplotypes, genomeLoc, haplotypes, - null, // called haplotypes + new HashSet<>(), // called haplotypes readLikelihoods); } From 3348855c6bd3f06d48f4136b497052cf4f8d6915 Mon Sep 17 00:00:00 2001 From: kvinter1 <42148952+kvinter1@users.noreply.github.com> Date: Wed, 22 Aug 2018 13:16:11 -0400 Subject: [PATCH 4/9] Update HaplotypeBAMDestination.java Deletion of redundant check of sourceReader being nonNull. (same as Line 45) --- .../hellbender/utils/haplotype/HaplotypeBAMDestination.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java index 9449d3bdace..8bbe68ba71d 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java @@ -43,7 +43,6 @@ protected HaplotypeBAMDestination( { Utils.nonNull(outPath, "outputPath cannot be null"); Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); - Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); Utils.nonNull(haplotypeReadGroupID, "haplotypeReadGroupID cannot be null"); this.haplotypeReadGroupID = haplotypeReadGroupID; From 3bc40a359072c0a5726289373acd10cd3a9a8838 Mon Sep 17 00:00:00 2001 From: kvinter Date: Wed, 22 Aug 2018 16:13:55 -0400 Subject: [PATCH 5/9] Fixed some formatting issues --- .../hellbender/utils/haplotype/HaplotypeBAMDestination.java | 2 +- .../hellbender/utils/haplotype/HaplotypeBAMWriter.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java index 8bbe68ba71d..a483b2e5ded 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java @@ -80,7 +80,7 @@ protected HaplotypeBAMDestination( public void add(final GATKRead read){ Utils.nonNull(read, "read cannot be null"); samWriter.addRead(read); - }; + } /** * Get the read group ID that is used by this writer when writing halpotypes as reads. diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index 7c58a3b367f..d89182d6078 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -50,7 +50,7 @@ public enum WriterType { * A mode for users. Writes out the reads aligned only to the called * haplotypes. Useful to understand why the caller is calling what it is */ - CALLED_HAPLOTYPES; + CALLED_HAPLOTYPES } @@ -144,7 +144,7 @@ public void writeReadsAlignedToHaplotypes(final Collection haplotypes * * @param read the read we want to write aligned to the reference genome, must not be null */ - protected void writeReadAgainstHaplotype(final GATKRead read) { + private void writeReadAgainstHaplotype(final GATKRead read) { Utils.nonNull(read, "read cannot be null"); output.add(read); } @@ -157,7 +157,7 @@ protected void writeReadAgainstHaplotype(final GATKRead read) { * be null * @param paddedReferenceLoc the genome loc of the padded reference, must not be null */ - protected void writeHaplotypesAsReads(final Collection haplotypes, + private void writeHaplotypesAsReads(final Collection haplotypes, final Set bestHaplotypes, final Locatable paddedReferenceLoc) { Utils.nonNull(haplotypes, "haplotypes cannot be null"); From 1ba7de709a2547547a1a3b82bcc8fce84c128571 Mon Sep 17 00:00:00 2001 From: kvinter Date: Tue, 28 Aug 2018 09:42:36 -0400 Subject: [PATCH 6/9] Reverted back to HaplotypeBAMDestination as abstract super and the existence of SAMFileDestination as its subclass. Kept the consolidation of HaplotypeBAMWriter and its subclasses (AllHaplotypeBAMWriter and CalledHaplotypeBAMWriter). --- .../haplotype/HaplotypeBAMDestination.java | 50 +++------------ .../utils/haplotype/HaplotypeBAMWriter.java | 2 +- .../utils/haplotype/SAMFileDestination.java | 61 +++++++++++++++++++ .../haplotype/HaplotypeBAMWriterUnitTest.java | 4 +- 4 files changed, 74 insertions(+), 43 deletions(-) create mode 100644 src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java index a483b2e5ded..4c439137a71 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMDestination.java @@ -3,24 +3,18 @@ import htsjdk.samtools.SAMFileHeader; import htsjdk.samtools.SAMProgramRecord; import htsjdk.samtools.SAMReadGroupRecord; -import java.nio.file.Path; -import org.broadinstitute.hellbender.utils.read.SAMFileGATKReadWriter; - import org.broadinstitute.hellbender.utils.Utils; import org.broadinstitute.hellbender.utils.read.GATKRead; -import org.broadinstitute.hellbender.utils.read.ReadUtils; import java.util.ArrayList; import java.util.List; /** - * Class used to direct output from a HaplotypeBAMWriter to a bam/sam file. + * Utility class that allows easy creation of destinations for the HaplotypeBAMWriters * */ - -public class HaplotypeBAMDestination { - private final SAMFileGATKReadWriter samWriter; +public abstract class HaplotypeBAMDestination { private final SAMFileHeader bamOutputHeader; private final String haplotypeReadGroupID; private final static String haplotypeSampleTag = "HC"; @@ -28,20 +22,10 @@ public class HaplotypeBAMDestination { /** * Create a new HaplotypeBAMDestination * - * @param outPath path where output is written (doesn't have to be local) - * @param createBamOutIndex true to create an index file for the bamout - * @param createBamOutMD5 true to create an md5 file for the bamout - * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination, must not be null - * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads + * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination. + * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads */ - protected HaplotypeBAMDestination( - final Path outPath, - final boolean createBamOutIndex, - final boolean createBamOutMD5, - final SAMFileHeader sourceHeader, - final String haplotypeReadGroupID) - { - Utils.nonNull(outPath, "outputPath cannot be null"); + protected HaplotypeBAMDestination(SAMFileHeader sourceHeader, final String haplotypeReadGroupID) { Utils.nonNull(sourceHeader, "sourceHeader cannot be null"); Utils.nonNull(haplotypeReadGroupID, "haplotypeReadGroupID cannot be null"); this.haplotypeReadGroupID = haplotypeReadGroupID; @@ -61,15 +45,6 @@ protected HaplotypeBAMDestination( bamOutputHeader.setReadGroups(readGroups); bamOutputHeader.addProgramRecord(new SAMProgramRecord("HalpotypeBAMWriter")); - - samWriter = new SAMFileGATKReadWriter(ReadUtils.createCommonSAMWriter( - outPath, - null, - getBAMOutputHeader(), // use the header derived from the source header by HaplotypeBAMDestination - false, - createBamOutIndex, - createBamOutMD5 - )); } /** @@ -77,10 +52,7 @@ protected HaplotypeBAMDestination( * * @param read the read to write out */ - public void add(final GATKRead read){ - Utils.nonNull(read, "read cannot be null"); - samWriter.addRead(read); - } + public abstract void add(final GATKRead read); /** * Get the read group ID that is used by this writer when writing halpotypes as reads. @@ -97,12 +69,10 @@ public void add(final GATKRead read){ public String getHaplotypeSampleTag() { return haplotypeSampleTag; } /** - * Close any resources associated with this destination. + * Close the destination + * */ - - void close(){ - samWriter.close(); - }; + abstract void close(); /** * Get the SAMFileHeader that is used for writing the output for this destination. @@ -112,4 +82,4 @@ public SAMFileHeader getBAMOutputHeader() { return bamOutputHeader; } -} +} \ No newline at end of file diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index d89182d6078..61b1baf0b58 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -71,7 +71,7 @@ public HaplotypeBAMWriter( final boolean createBamOutMD5, final SAMFileHeader sourceHeader) { - this(type, new HaplotypeBAMDestination(outputPath, createBamOutIndex, createBamOutMD5, sourceHeader, DEFAULT_HAPLOTYPE_READ_GROUP_ID)); + this(type, new SAMFileDestination(outputPath, createBamOutIndex, createBamOutMD5, sourceHeader, DEFAULT_HAPLOTYPE_READ_GROUP_ID)); } /** diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java new file mode 100644 index 00000000000..f4f5f69813f --- /dev/null +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/SAMFileDestination.java @@ -0,0 +1,61 @@ +package org.broadinstitute.hellbender.utils.haplotype; + +import htsjdk.samtools.SAMFileHeader; +import java.nio.file.Path; +import org.broadinstitute.hellbender.utils.read.GATKRead; +import org.broadinstitute.hellbender.utils.Utils; +import org.broadinstitute.hellbender.utils.read.ReadUtils; +import org.broadinstitute.hellbender.utils.read.SAMFileGATKReadWriter; + +import java.io.File; + +/** + * Class used to direct output from a HaplotypeBAMWriter to a bam/sam file. + */ +public final class SAMFileDestination extends HaplotypeBAMDestination { + private final SAMFileGATKReadWriter samWriter; + + /** + * Create a new file destination. + * + * @param outPath path where output is written (doesn't have to be local) + * @param createBamOutIndex true to create an index file for the bamout + * @param createBamOutMD5 true to create an md5 file for the bamout + * @param sourceHeader SAMFileHeader used to seed the output SAMFileHeader for this destination, must not be null + * @param haplotypeReadGroupID read group ID used when writing haplotypes as reads + */ + public SAMFileDestination( + final Path outPath, + final boolean createBamOutIndex, + final boolean createBamOutMD5, + final SAMFileHeader sourceHeader, + final String haplotypeReadGroupID) + { + super(sourceHeader, haplotypeReadGroupID); + samWriter = new SAMFileGATKReadWriter(ReadUtils.createCommonSAMWriter( + outPath, + null, + getBAMOutputHeader(), // use the header derived from the source header by HaplotypeBAMDestination + false, + createBamOutIndex, + createBamOutMD5 + )); + } + + /** + * Close any resources associated with this destination. + */ + @Override + void close() { samWriter.close(); } + + /** + * Write a read to the output file specified by this destination. + * + * @param read the read to write out, must not be null + */ + @Override + public void add(final GATKRead read) { + Utils.nonNull(read, "read cannot be null"); + samWriter.addRead(read); + } +} \ No newline at end of file diff --git a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java index 24d3a8044a4..3572673c24d 100644 --- a/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java +++ b/src/test/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriterUnitTest.java @@ -120,7 +120,7 @@ public Object[][] makeReadsLikelikhoodData() { ) throws IOException { final Path outPath = GATKBaseTest.createTempFile("haplotypeBamWriterTest", outputFileExtension).toPath(); - final HaplotypeBAMDestination fileDest = new HaplotypeBAMDestination(outPath, createIndex, createMD5, samHeader, "TestHaplotypeRG"); + final SAMFileDestination fileDest = new SAMFileDestination(outPath, createIndex, createMD5, samHeader, "TestHaplotypeRG"); try (final HaplotypeBAMWriter haplotypeBAMWriter = new HaplotypeBAMWriter(HaplotypeBAMWriter.WriterType.ALL_POSSIBLE_HAPLOTYPES, fileDest)) { haplotypeBAMWriter.writeReadsAlignedToHaplotypes( @@ -277,7 +277,7 @@ private class MockValidatingDestination extends HaplotypeBAMDestination { public boolean foundBases = false; // true we've seen a read that contains the expectedBaseSignature private MockValidatingDestination(String baseSignature) { - super(GATKBaseTest.createTempFile("haplotypeBamWriterTest", ".bam").toPath(), false, false, samHeader, "testGroupID"); + super(samHeader, "testGroupID"); expectedBaseSignature = baseSignature; } From b5fa08fe550490ea6916bc4f289deea579170ea8 Mon Sep 17 00:00:00 2001 From: kvinter Date: Thu, 30 Aug 2018 09:07:01 -0400 Subject: [PATCH 7/9] Attempt to fix @cmnbroad 's corrections' --- .../utils/haplotype/HaplotypeBAMWriter.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index 61b1baf0b58..97bc189facf 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -32,7 +32,7 @@ public class HaplotypeBAMWriter implements AutoCloseable { private static final int bestHaplotypeMQ = 60; private static final int otherMQ = 0; - protected final HaplotypeBAMDestination output; + private final HaplotypeBAMDestination output; private WriterType writerType; private boolean writeHaplotypes = true; @@ -104,12 +104,14 @@ public void close() { /** * Write out a BAM representing for the haplotype caller at this site * writerType (ALL_POSSIBLE_HAPLOTYPES or CALLED_HAPLOTYPES) determines inputs to writeHaplotypesAsReads + * Write out a BAM representing the haplotypes at this site, based on the value for writerType used when + * the writer was constructed (ALL_POSSIBLE_HAPLOTYPES or CALLED_HAPLOTYPES). * - * @param haplotypes a list of all possible haplotypes at this loc - * @param paddedReferenceLoc the span of the based reference here - * @param bestHaplotypes a list of the best (a subset of all) haplotypes that actually went forward into genotyping - * @param calledHaplotypes a list of the haplotypes that where actually called as non-reference - * @param readLikelihoods a map from sample -> likelihoods for each read for each of the best haplotypes + * @param haplotypes a list of all possible haplotypes at this loc, cannot be null + * @param paddedReferenceLoc the span of the based reference here, cannot be null + * @param bestHaplotypes a list of the best (a subset of all) haplotypes that actually went forward into genotyping, cannot be null + * @param calledHaplotypes a list of the haplotypes that where actually called as non-reference, cannot be null + * @param readLikelihoods a map from sample -> likelihoods for each read for each of the best haplotypes, cannot be null */ public void writeReadsAlignedToHaplotypes(final Collection haplotypes, final Locatable paddedReferenceLoc, @@ -121,13 +123,19 @@ public void writeReadsAlignedToHaplotypes(final Collection haplotypes Utils.nonNull(paddedReferenceLoc, "paddedReferenceLoc cannot be null"); Utils.nonNull(calledHaplotypes, "calledHaplotypes cannot be null"); Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); + Utils.nonNull(bestHaplotypes, "bestHaplotypes cannot be null"); if (calledHaplotypes.isEmpty() && writerType.equals(WriterType.CALLED_HAPLOTYPES)) { // only write out called haplotypes return; } - Collection haplotypesToWrite = writerType.equals(WriterType.CALLED_HAPLOTYPES) ? calledHaplotypes : haplotypes; - Set bestHaplotypesToWrite = writerType.equals(WriterType.CALLED_HAPLOTYPES) ? calledHaplotypes : new LinkedHashSet<>(bestHaplotypes); + Collection haplotypesToWrite = haplotypes; + Set bestHaplotypesToWrite = new LinkedHashSet<>(bestHaplotypes); + + if (writerType.equals(WriterType.CALLED_HAPLOTYPES)){ + haplotypesToWrite = calledHaplotypes; + bestHaplotypesToWrite = calledHaplotypes; + } writeHaplotypesAsReads(haplotypesToWrite, bestHaplotypesToWrite, paddedReferenceLoc); From 222ecc0d5715b3a4bc45ebc1e2e5241161ecf7ff Mon Sep 17 00:00:00 2001 From: kvinter Date: Thu, 30 Aug 2018 10:00:52 -0400 Subject: [PATCH 8/9] New changes to comments --- .../hellbender/utils/haplotype/HaplotypeBAMWriter.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index 97bc189facf..b158f6e33bc 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -125,14 +125,13 @@ public void writeReadsAlignedToHaplotypes(final Collection haplotypes Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); Utils.nonNull(bestHaplotypes, "bestHaplotypes cannot be null"); - if (calledHaplotypes.isEmpty() && writerType.equals(WriterType.CALLED_HAPLOTYPES)) { // only write out called haplotypes - return; - } - Collection haplotypesToWrite = haplotypes; Set bestHaplotypesToWrite = new LinkedHashSet<>(bestHaplotypes); if (writerType.equals(WriterType.CALLED_HAPLOTYPES)){ + if (calledHaplotypes.isEmpty()){ + return; + } haplotypesToWrite = calledHaplotypes; bestHaplotypesToWrite = calledHaplotypes; } From 893d5576164705933cf1990bb35f1a41b828c2e5 Mon Sep 17 00:00:00 2001 From: kvinter Date: Thu, 30 Aug 2018 10:19:05 -0400 Subject: [PATCH 9/9] Changes to if statement in HaplotypeBAMWriter --- .../utils/haplotype/HaplotypeBAMWriter.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java index b158f6e33bc..2152429dd3a 100644 --- a/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java +++ b/src/main/java/org/broadinstitute/hellbender/utils/haplotype/HaplotypeBAMWriter.java @@ -125,18 +125,15 @@ public void writeReadsAlignedToHaplotypes(final Collection haplotypes Utils.nonNull(readLikelihoods, "readLikelihoods cannot be null"); Utils.nonNull(bestHaplotypes, "bestHaplotypes cannot be null"); - Collection haplotypesToWrite = haplotypes; - Set bestHaplotypesToWrite = new LinkedHashSet<>(bestHaplotypes); - if (writerType.equals(WriterType.CALLED_HAPLOTYPES)){ if (calledHaplotypes.isEmpty()){ return; } - haplotypesToWrite = calledHaplotypes; - bestHaplotypesToWrite = calledHaplotypes; - } + writeHaplotypesAsReads(calledHaplotypes, calledHaplotypes, paddedReferenceLoc); - writeHaplotypesAsReads(haplotypesToWrite, bestHaplotypesToWrite, paddedReferenceLoc); + } else { + writeHaplotypesAsReads(haplotypes, new LinkedHashSet<>(bestHaplotypes), paddedReferenceLoc); + } final int sampleCount = readLikelihoods.numberOfSamples(); for (int i = 0; i < sampleCount; i++) {