JaneliaSciComp / msg

Multiplexed Shotgun Genotyping
http://genomics.princeton.edu/AndolfattoLab/MSG.html
11 stars 12 forks source link

Samtools location #25

Open lparsons opened 12 years ago

lparsons commented 12 years ago

This incorporates Tina's changes to use a samtools binary that must be in the msg directory, and Lance's change to check for the specific version of samtools.

JaneliaSciComp commented 12 years ago

Thanks Lance!

On 2/10/12 11:29 AM, "Lance Parsons" <reply+i-3174305-6dba72c1c6bad935a6a22cabcfcd5cbfb479e94d-1020012@reply.git hub.com> wrote:

This incorporates Tina's changes to use a samtools binary that must be in the msg directory, and Lance's change to check for the specific version of samtools.

You can merge this Pull Request by running:

git pull https://github.com/lparsons/msg samtools-location

Or you can view, comment on it, or merge it online at:

https://github.com/JaneliaSciComp/msg/pull/25

-- Commit Summary --

  • Calling samtools from within the msg directory
  • Added version checking for samtools dependency

-- File Changes --

M make-pileups.sh (14) M msg.pl (14) M test_dependencies_executable.sh (12)

-- Patch Links --

https://github.com/JaneliaSciComp/msg/pull/25.patch https://github.com/JaneliaSciComp/msg/pull/25.diff


Reply to this email directly or view it on GitHub: https://github.com/JaneliaSciComp/msg/pull/25

lparsons commented 12 years ago

So we just ran into an issue with this again, since the latest versions of samtools no longer support the pileup command. The current workaround is to set the users PATH to point to the appropriate version of samtools prior to running MSG.

While incorporating this fix would be OK, it is not ideal as it does complicate the install process. It would be nice if there was a configuration option that allowed someone to point to the binary to use for samtools directly (preferable). Alternatively, you could consider distributing samtools (it is licensed under the MIT license), though that might just be a headache.

Finally, regardless of other options, it should be possible to make it compatible with mpileup since that is capable of producing nearly the same files with the appropriate options used.

gregpinero commented 12 years ago

Hi Lance.

I was leaning towards including samtools.

I looked into converting to mpileup but it doesn't give us the 10 column pileup format so it didn't look possible without changing a lot of MSG.

lparsons commented 12 years ago

That would certainly work since it seems that MSG is very dependent on the version. If you are going that route, perhaps you do want to pull this change?

My immediate issue is that it appears that setting the users PATH does not work since qsub will use a default path of /usr/local/bin:/usr/ucb:/bin:/usr/bin. Any thoughts on how I can get this working for a user on our cluster (which has a recent version of samtools installed into /usr/local/bin)?

Thanks.

gregpinero commented 12 years ago

Yeah, I think I will pull this change.

PATH works for us. There's an option in qsub to use the current environment. MSG already includes that option in its qsub calls. I'm not sure why that wouldn't be working for you.

If you get the latest version of MSG from this repo, I have it print out all of the qsub calls from msgCluster.pl. That might be a good starting place for debugging.

gregpinero commented 12 years ago

At least on our cluster it's: -V - Transfer all your environment variables to the job's environment.

I think that's standard though.

lparsons commented 12 years ago

Hmmm.. Thanks, I didn't realize that. I'll do a bit more digging into the issue as the PATH setting should indeed fix things. Thanks.

On Wed, May 2, 2012 at 1:46 PM, Greg Pinero < reply@reply.github.com

wrote:

At least on our cluster it's: -V - Transfer all your environment variables to the job's environment.

I think that's standard though.


Reply to this email directly or view it on GitHub: https://github.com/JaneliaSciComp/msg/pull/25#issuecomment-5467580