Skip to content
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

Add ID specific to each tool/command which Machine.t can dispatch on #213

Closed
ihodes opened this issue Mar 30, 2016 · 9 comments · Fixed by #234
Closed

Add ID specific to each tool/command which Machine.t can dispatch on #213

ihodes opened this issue Mar 30, 2016 · 9 comments · Fixed by #234

Comments

@ihodes
Copy link
Member

ihodes commented Mar 30, 2016

Sometimes we (pipeline writers) know better than Biokepi writers.

@smondet
Copy link
Member

smondet commented Mar 30, 2016

But the pipeline-writers can already do whatever they want with the requirements, through the Machine.t they build. What use-case would be missing?

@ihodes
Copy link
Member Author

ihodes commented Mar 30, 2016

If a tool says it needs "small memory" and I know that for my use case this tool (but not others specifying "small memory" needs 10GB, can I do that in Machine?

@smondet
Copy link
Member

smondet commented Mar 31, 2016

@ihodes in that case, it's a “bug” in that module. Two steps:

  1. copy the workflow-making function to your own code, fix it, and use it right away.
  2. submit a PR to biokepi that fixes it for everybody.

The only thing right now, is that Pipeline.t is not extensible; but this should change very soon.

On top of that, if useful, we can pass one more argument (or maybe part of the “requirements” which should be renamed then “self-description”) to the run_function that is unique to each tool; then the implementation of the Machine.t can do something special for that particular tool?
What do you think?

@ihodes
Copy link
Member Author

ihodes commented Mar 31, 2016

  1. copy the workflow-making function to your own code, fix it, and use it right away.

Doable, but way hackier than just passing a list of your own options in... this seems like an untenable solution.

  1. submit a PR to biokepi that fixes it for everybody.

Definitely ideal if the solution is clear & general-purpose. I'm thinking it's not unlikely to know what what works for your situation, but that may not be the best case for everyone using Biokepi.

On top of that, if useful, we can pass one more argument (or maybe part of the “requirements” which should be renamed then “self-description”) to the run_function that is unique to each tool; then the implementation of the Machine.t can do something special for that particular tool?

I like this idea, but if anything I think it maybe should be in addition to just exposing ~requirements.

@smondet
Copy link
Member

smondet commented Mar 31, 2016

  1. copy the workflow-making function to your own code, fix it, and use it right away.

Doable, but way hackier than just passing a list of your own options in... this seems like an untenable solution.

No it's not really “hacky” that's the goal of using a library.

cf. things like #146:
replacing the installation of Mosaik locally.

  1. submit a PR to biokepi that fixes it for everybody.

Definitely ideal if the solution is clear & general-purpose. I'm thinking it's not unlikely to know what what works for your situation, but that may not be the best case for everyone using Biokepi.

On top of that, if useful, we can pass one more argument (or maybe part of the “requirements” which should be renamed then “self-description”) to the run_function that is unique to each tool; then the implementation of the Machine.t can do something special for that particular tool?

I like this idea, but if anything I think it maybe should be in addition to just exposing ~requirements.

the problem is also “why just ~requirements?” we could pass many more options
likes overriding more things (the ~make, the ~name, etc.)?

passing an identifier of the tools in requirements is less hacky, also more future-proof

The goal of the ~run_with:Machine.t is to put common things together an not
copy around 10 error-prone arguments to each function. We actually want to get rid the ~processors to simplify the signatures also.

@ihodes
Copy link
Member Author

ihodes commented Mar 31, 2016

passing an identifier of the tools in requirements is less hacky, also more future-proof

I'm happy as long as we have an ID we can dispatch on in the Machine.t, then I can do what I want with it there.

@ihodes ihodes changed the title All Biokepi tools should take an optional ?requirements which overrwrites the defaults Add ID specific to each tool/command which Machine.t can dispatch on Mar 31, 2016
@ihodes
Copy link
Member Author

ihodes commented Apr 13, 2016

I'd envision the Machine.t, in an ideal world, to come from another library (like Hamkepi, but say, one specific to AWS) and tweaking memory requirements by modifying an external library seems suboptimal.

@smondet
Copy link
Member

smondet commented Apr 13, 2016

So I'm adding facilities for tools to self-identify themselves so that the user creating a Machine.t can override precisely whatever they want.
If the Machine.t comes from a library it is the job of that library to expose (or hide) that hackability.

@ihodes
Copy link
Member Author

ihodes commented Apr 13, 2016

If the Machine.t comes from a library it is the job of that library to expose (or hide) that hackability.

Seems like a reasonable compromise 👍 (and something we should add to Hamkepi as soon as this issue is in)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants