Glen Beane wrote: > We recently updated to the latest galaxy-dist, and learned that the sam_merge.xml tool now uses picard MergeSamFiles.jar to merge the files instead of the samtools merge wrapper sam_merge.py. > > this is a problem for us because MergeSamFiles.jar does not honor $TMPDIR when creating temporary file names (the jvm developers inexplicably hard code the value of java.io.tmpdir to /tmp in Unix/Linux rather than doing the Right Thing) . On our cluster, TMPDIR is set to something like /scratch/batch_job_id/. This location has plenty of free space, however /tmp does not and now we can't successfully merge largeish bam files. > > In case anyone else is bit by this, I think there are two options > > the Picard tools take an optional TMP_DIR= argument that lets us specify the location we want to use for a temporary directory. Initially we ended up modifying the .xml to add TMP_DIR=\$TMPDIR to the arguments to MergeSamFiles.jar. This works, but we could potentially need to do this with multiple Picard tools and not just MergeSamFiles. Now I am probably going to go with the following solution: > > add something like "export _JAVA_OPTIONS=-Djava.io.tmpdir=$TMPDIR" to the .bashrc file for my Galaxy user. This is what I've just done on our local cluster as well. I was also confounded by the lack of a proper environment variable to do this.

For what it's worth, I ran into an issue with the use of /tmp as well. When merging a lot of BAM files, /tmp filled up and the merge failed. To make matters worse, since STDERR is redirected and the exit status of java is not checked, the item in the Galaxy history appeared OK. Though examination of the .log file revealed the issue, users weren't aware of the problem right away, causing further confusion. To get around this, I implemented two fixes: 1 - Wrapper for sam_merge2 very similar to hide_stderr.py used by the ncbi_blast_plus tools. I did have to "hack" it a bit, however, since when I output the captured stderr, galaxy would hang if it was too large. My "fix" was to output the first and last 7500 characters of stderr (truncating the middle). Now when the merge fails, users see a failure in Galaxy (red history item). 2 - Looking at other Picard tools, it seems that at least some of them explicitly set TMP_DIR to $__new_files_directory__, which in this case seemed appropriate, at least for my use case. So I added that simple change to the sam_merge.xml command line. I'm open to better solutions if people see issues with these, but I would suggest that Galaxy be updated to include fixes for these issues since it seems others are likely to run into these problems. If people generally agree I'd be happy to provide create tickets and/or provide changesets or whatever the Galaxy maintainers would prefer.

Lance Glen Beane wrote: >On Sep 27, 2011, at 1:14 PM, Nate Coraor wrote: > >>Glen Beane wrote: >>>We recently updated to the latest galaxy-dist, and learned that the sam_merge.xml tool now uses picard MergeSamFiles.jar to merge the files instead of the samtools merge wrapper sam_merge.py. >>> >>>this is a problem for us because MergeSamFiles.jar does not honor $TMPDIR when creating temporary file names (the jvm developers inexplicably hard code the value of java.io.tmpdir to /tmp in Unix/Linux rather than doing the Right Thing) . On our cluster, TMPDIR is set to something like /scratch/batch_job_id/. This location has plenty of free space, however /tmp does not and now we can't successfully merge largeish bam files. >>> >>>In case anyone else is bit by this, I think there are two options >>> >>>the Picard tools take an optional TMP_DIR= argument that lets us specify the location we want to use for a temporary directory. Initially we ended up modifying the .xml to add TMP_DIR=\$TMPDIR to the arguments to MergeSamFiles.jar. This works, but we could potentially need to do this with multiple Picard tools and not just MergeSamFiles. Now I am probably going to go with the following solution: >>> >>>add something like "export _JAVA_OPTIONS=-Djava.io.tmpdir=$TMPDIR" to the .bashrc file for my Galaxy user. >>This is what I've just done on our local cluster as well. I was also >>confounded by the lack of a proper environment variable to do this. >> > >It looks like the export _JAVA_OPTIONS=-Djava.io.tmpdir=$TMPDIR solution breaks some tools (like snpEFF, which is a 3rd party tool we use). The Jvm prints a diagnostic message to stderr that looks something like this: > >Picked up _JAVA_OPTIONS: -Djava.io.tmpdir=/scratch/32095.scyld.localdomain > >so in this case the tool fails, since it does not have a wrapper. The sam_merge.xml tool redirects stderr, so it doesn't have this problem. I am thinking about putting a wrapper script for java in my galaxy user's path that adds -Djava.io.tmpdir=$TMPDIR to the arguments. > > > >-- >Glen L. Beane >Senior Software Engineer >The Jackson Laboratory >(207) 288-6153 > > >___________________________________________________________ >Please keep all replies on the list by using "reply all" >in your mail client. To manage your subscriptions to this >and other Galaxy lists, please use the interface at: > > http://lists.bx.psu.edu/ -- Lance Parsons - Scientific Programmer 134 Carl C. Icahn Laboratory Lewis-Sigler Institute for Integrative Genomics Princeton University

The patches have been put into a pull request.

Lance Nate Coraor wrote: > > Lance Parsons wrote: >> For what it's worth, I ran into an issue with the use of /tmp as >> well. When merging a lot of BAM files, /tmp filled up and the merge >> failed. To make matters worse, since STDERR is redirected and the >> exit status of java is not checked, the item in the Galaxy history >> appeared OK. Though examination of the .log file revealed the >> issue, users weren't aware of the problem right away, causing >> further confusion. To get around this, I implemented two fixes: >> >> 1 - Wrapper for sam_merge2 very similar to hide_stderr.py used by >> the ncbi_blast_plus tools. I did have to "hack" it a bit, however, >> since when I output the captured stderr, galaxy would hang if it was >> too large. My "fix" was to output the first and last 7500 >> characters of stderr (truncating the middle). Now when the merge >> fails, users see a failure in Galaxy (red history item). >> >> 2 - Looking at other Picard tools, it seems that at least some of >> them explicitly set TMP_DIR to $__new_files_directory__, which in >> this case seemed appropriate, at least for my use case. So I added >> that simple change to the sam_merge.xml command line. >> >> I'm open to better solutions if people see issues with these, but I >> would suggest that Galaxy be updated to include fixes for these >> issues since it seems others are likely to run into these problems. >> If people generally agree I'd be happy to provide create tickets >> and/or provide changesets or whatever the Galaxy maintainers would >> prefer. > > Hi Lance, > > Could you put these patches in an issue (or create a pull request)? > > Thanks, > --nate > > -- Lance Parsons - Scientific Programmer 134 Carl C. Icahn Laboratory Lewis-Sigler Institute for Integrative Genomics Princeton University