From 27028388d51e405fed515185b6cb76d4ac8eda22 Mon Sep 17 00:00:00 2001 From: Len Trigg Date: Fri, 17 Mar 2017 10:51:25 +1300 Subject: [PATCH 1/2] Obey the tmpDir setting in several constructors that currently ignore it. As part of this, made one constructor follow the existing convention of calling initWriter() (This private method had an unused parameter, which has been removed), and ensured that both initWriter and initializeBAMWriter both call setTempDirectory() when needed. --- .../java/htsjdk/samtools/SAMFileWriterFactory.java | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java b/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java index b788fbe89..265e0ae19 100644 --- a/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java +++ b/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java @@ -253,7 +253,6 @@ public SAMFileWriter makeBAMWriter(final SAMFileHeader header, final boolean pre if (this.createIndex && !createIndex) { log.warn("Cannot create index for BAM because output file is not a regular file: " + outputFile.getAbsolutePath()); } - if (this.tmpDir != null) ret.setTempDirectory(this.tmpDir); initializeBAMWriter(ret, header, presorted, createIndex); if (this.useAsyncIo) return new AsyncSAMFileWriter(ret, this.asyncOutputBufferSize); @@ -268,6 +267,7 @@ private void initializeBAMWriter(final BAMFileWriter writer, final SAMFileHeader if (maxRecordsInRam != null) { writer.setMaxRecordsInRam(maxRecordsInRam); } + if (this.tmpDir != null) writer.setTempDirectory(this.tmpDir); writer.setHeader(header); if (createIndex && writer.getSortOrder().equals(SAMFileHeader.SortOrder.coordinate)) { writer.enableBamIndexConstruction(); @@ -294,14 +294,7 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre ? new SAMTextWriter(new Md5CalculatingOutputStream(new FileOutputStream(outputFile, false), new File(outputFile.getAbsolutePath() + ".md5")), samFlagFieldOutput) : new SAMTextWriter(outputFile, samFlagFieldOutput); - ret.setSortOrder(header.getSortOrder(), presorted); - if (maxRecordsInRam != null) { - ret.setMaxRecordsInRam(maxRecordsInRam); - } - ret.setHeader(header); - - if (this.useAsyncIo) return new AsyncSAMFileWriter(ret, this.asyncOutputBufferSize); - else return ret; + return initWriter(header, presorted, ret); } catch (final IOException ioe) { throw new RuntimeIOException("Error opening file: " + outputFile.getAbsolutePath()); } @@ -324,7 +317,7 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre if (samFlagFieldOutput == SamFlagField.NONE) { samFlagFieldOutput = Defaults.SAM_FLAG_FIELD_FORMAT; } - return initWriter(header, presorted, false, new SAMTextWriter(stream, samFlagFieldOutput)); + return initWriter(header, presorted, new SAMTextWriter(stream, samFlagFieldOutput)); } /** @@ -338,24 +331,23 @@ public SAMFileWriter makeSAMWriter(final SAMFileHeader header, final boolean pre */ public SAMFileWriter makeBAMWriter(final SAMFileHeader header, final boolean presorted, final OutputStream stream) { - return initWriter(header, presorted, true, new BAMFileWriter(stream, null, this.getCompressionLevel(), this.deflaterFactory)); + return initWriter(header, presorted, new BAMFileWriter(stream, null, this.getCompressionLevel(), this.deflaterFactory)); } /** * Initialize SAMTextWriter or a BAMFileWriter and possibly wrap in AsyncSAMFileWriter - * * @param header entire header. Sort order is determined by the sortOrder property of this arg. * @param presorted if true, SAMRecords must be added to the SAMFileWriter in order that agrees with header.sortOrder. - * @param binary do we want to generate a BAM or a SAM * @param writer SAM or BAM writer to initialize and maybe wrap. */ - private SAMFileWriter initWriter(final SAMFileHeader header, final boolean presorted, final boolean binary, + private SAMFileWriter initWriter(final SAMFileHeader header, final boolean presorted, final SAMFileWriterImpl writer) { writer.setSortOrder(header.getSortOrder(), presorted); if (maxRecordsInRam != null) { writer.setMaxRecordsInRam(maxRecordsInRam); } + if (this.tmpDir != null) writer.setTempDirectory(this.tmpDir); writer.setHeader(header); if (this.useAsyncIo) return new AsyncSAMFileWriter(writer, this.asyncOutputBufferSize); From 19961c9e482342e712c2d98f29ee725917c70acf Mon Sep 17 00:00:00 2001 From: Len Trigg Date: Mon, 27 Mar 2017 12:01:46 +1300 Subject: [PATCH 2/2] Test that setMaxRecordsInRam and setTempDirectory settings on SAMFileWriterFactory make it to the writer implementation --- .../java/htsjdk/samtools/SAMFileWriterFactory.java | 16 +++++++++++++++ .../java/htsjdk/samtools/SAMFileWriterImpl.java | 10 ++++++++- .../htsjdk/samtools/SAMFileWriterFactoryTest.java | 24 +++++++++++++++++++++- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java b/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java index 265e0ae19..30b36d7b3 100644 --- a/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java +++ b/src/main/java/htsjdk/samtools/SAMFileWriterFactory.java @@ -174,6 +174,14 @@ public SAMFileWriterFactory setMaxRecordsInRam(final int maxRecordsInRam) { } /** + * Gets the maximum number of records held in RAM before spilling to disk during sorting. + * @see #setMaxRecordsInRam(int) + */ + public int getMaxRecordsInRam() { + return maxRecordsInRam; + } + + /** * Turn on or off the use of asynchronous IO for writing output SAM and BAM files. If true then * each SAMFileWriter creates a dedicated thread which is used for compression and IO activities. */ @@ -211,6 +219,14 @@ public SAMFileWriterFactory setTempDirectory(final File tmpDir) { } /** + * Gets the temporary directory that will be used when sorting data. + * @see #setTempDirectory(File) + */ + public File getTempDirectory() { + return tmpDir; + } + + /** * Set the flag output format only when writing text. * Default value: [[htsjdk.samtools.SAMTextWriter.samFlagFieldOutput.DECIMAL]] */ diff --git a/src/main/java/htsjdk/samtools/SAMFileWriterImpl.java b/src/main/java/htsjdk/samtools/SAMFileWriterImpl.java index 5e0ecdb45..31a8604dc 100644 --- a/src/main/java/htsjdk/samtools/SAMFileWriterImpl.java +++ b/src/main/java/htsjdk/samtools/SAMFileWriterImpl.java @@ -111,7 +111,11 @@ void setMaxRecordsInRam(final int maxRecordsInRam) { } this.maxRecordsInRam = maxRecordsInRam; } - + + int getMaxRecordsInRam() { + return maxRecordsInRam; + } + /** * When writing records that are not presorted, specify the path of the temporary directory * for spilling to disk. Must be called before setHeader(). @@ -123,6 +127,10 @@ void setTempDirectory(final File tmpDir) { } } + File getTempDirectory() { + return tmpDir; + } + /** * Must be called before addAlignment. Header cannot be null. */ diff --git a/src/test/java/htsjdk/samtools/SAMFileWriterFactoryTest.java b/src/test/java/htsjdk/samtools/SAMFileWriterFactoryTest.java index 8c15a1656..0b8d7b5ac 100644 --- a/src/test/java/htsjdk/samtools/SAMFileWriterFactoryTest.java +++ b/src/test/java/htsjdk/samtools/SAMFileWriterFactoryTest.java @@ -167,7 +167,29 @@ private void createSmallBamToOutputStream(final OutputStream outputStream,boolea fillSmallBam(writer); writer.close(); } - + + @Test(description="check that factory settings are propagated to writer") + public void testFactorySettings() throws Exception { + final SAMFileWriterFactory factory = new SAMFileWriterFactory(); + factory.setCreateIndex(false); + factory.setCreateMd5File(false); + final File wontBeUsed = new File("wontBeUsed.tmp"); + final int maxRecsInRam = 271828; + factory.setMaxRecordsInRam(maxRecsInRam); + factory.setTempDirectory(wontBeUsed); + final SAMFileHeader header = new SAMFileHeader(); + header.setSortOrder(SAMFileHeader.SortOrder.coordinate); + header.addSequence(new SAMSequenceRecord("chr1", 123)); + try (final SAMFileWriter writer = factory.makeBAMWriter(header, false, new ByteArrayOutputStream())) { + Assert.assertEquals(maxRecsInRam, ((SAMFileWriterImpl) writer).getMaxRecordsInRam()); + Assert.assertEquals(wontBeUsed, ((SAMFileWriterImpl) writer).getTempDirectory()); + } + try (final SAMFileWriter writer = factory.makeSAMWriter(header, false, new ByteArrayOutputStream())) { + Assert.assertEquals(maxRecsInRam, ((SAMFileWriterImpl) writer).getMaxRecordsInRam()); + Assert.assertEquals(wontBeUsed, ((SAMFileWriterImpl) writer).getTempDirectory()); + } + } + private int fillSmallBam(SAMFileWriter writer) { final SAMRecordSetBuilder builder = new SAMRecordSetBuilder(); builder.addUnmappedFragment("HiMom!");