-
Notifications
You must be signed in to change notification settings - Fork 196
Container recipe for wrappers #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I am transferring this as I would like to reignite the conversation. Would you like some help with this @johanneskoester (that is if you think this feature should be added)? If so, would you be able to point me towards where would be the best place for me to start investigating a solution? |
Thanks Mike |
Great! Thanks @vsoch . And yes, I did see the message from Johannes - the poor guy is going to have so many notifications to come back to. Did you want to have a chat to sort out how best to approach implementing this? What is the best way for communicating for you? (maybe we can continue this via DM on twitter? or anywhere else is fine) |
I wouldn't be the right one to have an idea for best practices for implementation, I have worked on snakemake source code but not snakemake-wrappers source code. If you think you have enough experience to be that guide, then we can definitely give it a go and then get Johannes feeback! But otherwise, I think we should at least wait for @johanneskoester to come back and give some vision for how it's best done. And reaching me - yes DM on Twitter works, as does any channel on Gitter. |
Ok, fair point. We'll wait for Johannes. |
@johanneskoester would you be interested in this? |
Hi folks, All in all, just a few lines of code I think. Which makes me even more sorry to not having done this myself already. |
@mbhall88 do you have an example of the wrapper meta yaml so I could see what it looks like? I see that the dag also pulls singularity containers and I'm trying to figure out how those two are different. |
@johanneskoester are you suggesting that if the container URI is in the @vsoch I'll provide an example from
name: "bwa mem"
description: Map reads using bwa mem, with optional sorting using
samtools or picard.
authors:
- Johannes Köster
- Julian de Ruiter So I guess we could just add something akin to |
@mbhall88 ah I think I understand now! So the wrapper would essentially have a singularity (via docker) image backend instead of using conda. @mbhall88 what about if a recipe would want to provide a conda and container option? Would we want to maintain separate wrappers, e.g.::
or even a different "container" namespace that mirrors the top level:
Or add some parameter to the recipe for the user to choose (and then have a default?). My worry with doing the first is that the user would expect the exact same outcome / performance between the container and conda runs, and I'm not sure we could promise that. On the other hand, maintaining them separately is also challenging. What do you think? |
I was thinking it might be good to provide both a conda environment (if applicable) and a container for the wrapper. Then, we decide which environment to use based on the snakemake CLI options
This way the user doesn't have to think about additional parameters within the snakefile and it is all controlled with existing CLI options that are already required for using wrappers and containers. |
I like that too! And probably since most wrappers are conda now, we would default to that if a container isn't available. And this also means we would try our best to match versions, but of course it can't be absolutely guaranteed that the container and conda install would be exactly the same. One more question for you @mbhall88 before we look at code - I'm concerned about mapping containers to snakemake-wrappers. For example, if you look at the biocontainers bwa Dockerfile, it's only installing bwa, but the wrapper that you linked has bwa, samtools, and picard. We should decide on some strategy for mapping containers to wrappers.
|
Yes, you're right. This is a pain point. |
Sort of both - we would have a Dockerfile that uses the environment.yml to install stuffs. That way, they are as close to identical as possible! The one issue I see for setting up automated builds is the lack of modularity in the single repo. Ideally we would have one automated build per repo without some complicated logic for the trigger and settings. I was thinking we should slowly port over to a Snakemake-wrappers org, but of course we need feedback from @johanneskoester first! But if y’all like the idea I can take a first shot at a modular repo with an automated build and then work on a Snakemake PR to add the functionality. |
@mbhall88 one more question - does this approach assume we need to read in the meta.yaml file to discover the container? Or did you have something else in mind? I did a grep of snakemake and (as far as I can tell) it's not read in anywhere, because largely we don't need it for the run. |
hey @mbhall88 - happy Saturday! I got us started on something to get this ball rolling, see snakemake/snakemake#532. Note that the implementation will require changes to the wrappers repository here, and I've detailed what we need to discuss in the PR description. |
@vsoch you are impressive!! I have some questions/comments but will move the discussion to the PR |
Transferring issue from bitbucket.
Adrien Leger:
Johannes:
Adrien:
Johannes:
The text was updated successfully, but these errors were encountered: