search

matsengrp / phip-flow

A Nextflow pipeline to align, merge, and organize large PhIP-Seq datasets
MIT License
11 stars 6 forks source link

Adding support for local singularity and apptainer profiles #73

Closed afonsoguerra closed 4 months ago

afonsoguerra commented 8 months ago

First of all, let me thank you for your work on this pipeline. It really integrates a series of useful tools in a neat package.

As I'm sure you're aware, Docker is not universally used because of the lack of sandboxing and ease of privilege escalation. For my application, I rather use Apptainer, which is a recent open-source fork of singularity. This pull request creates two new local profiles to allow these tools to be used.

Very few modifications were made, as this will automatically use the Docker containers that are already specified.

I have run the basic tests of the pipeline and it seems to be working fine.

Please consider the inclusion of this pull request in your code and keep up the good work.

Kind regards, José Afonso Guerra-Assuncao

jgallowa07 commented 4 months ago

Sorry for the very late reply here @afonsoguerra. Thanks for the PR! This seems like a reasonable addition, though I'm not sure I see the reason to mount ${HOME} to ${PWD}. Can you explain the logic there?

afonsoguerra commented 4 months ago

Hey, thanks for coming back to me on this. It isn't mounting one onto the other, the logic is simply to make them both visible in their original locations inside the container.

By default HOME should be mounted automatically, but my home is on a different filesystem and it wasn't quite working well without explicit bind (likely a singularity issue in my particular system, but I can't update the system). As for the current working directory (PWD), that's where I have my files and want to run it!

Further info available in: https://apptainer.org/docs/user/main/bind_paths_and_mounts.html

jgallowa07 commented 4 months ago

Thanks for the explanation. This seems to work fine on our local cluster so I'm not against merging this in. @sminot Will this mess with anything on your end?

sminot commented 4 months ago

This won't be an issue on my end at all. Adding profiles is always nice because they don't do anything unless the user selects them. When I run this on the cluster I set mostly all of the same params, just using a personal config instead of a profile. The issue that we really have to worry about here is setting the Apptainer folder so that it doesn't download a bunch of images to our home directory, but that's more institution specific and so I didn't see a need to put it in the repo.

Thumbs up from me. And I'm glad to hear that this is useful for someone on the other side of the pond!

jgallowa07 commented 4 months ago

Great. Thanks @afonsoguerra for the contribution!