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

Maker: update 2.31.10 + add support for mpi #2149

Merged
merged 3 commits into from
May 3, 2019

Conversation

abretaud
Copy link
Contributor

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

FOR REVIEWER:

  • .shed.yml file ok
    • Toolshed user iuc has access to associated toolshed repo(s)
  • Indentation is correct (4 spaces)
  • Tool version/build ok
  • <command/>
    • Text parameters, input and output files 'single quoted'
    • Use of <![CDATA[ ... ]]> tags
    • Parameters of type text or having optional="true" attribute are checked with if str($param) before being used
  • Data parameters have a format attribute containing datatypes recognised by Galaxy
  • Tests
    • Parameters are reasonably covered
    • Test files are appropriate
  • Help
    • Valid restructuredText and uses <![CDATA[ ... ]]> tags
  • Complies with other best practice in Best Practices Doc

A minor update for maker, and also added support for MPI (finally found a way to avoid systematic segfault).
I'm a little nervous with the MPI stuff, but tests are ok (= in a non-cluster environment) and it seems to work fine on our cluster with a properly configure job destination.

@abretaud
Copy link
Contributor Author

(There's also this PR which should improve support for mpi on SGE cluster: conda-forge/openmpi-feedstock#30)

@abretaud
Copy link
Contributor Author

humpf. maybe mpi is not so simple. we should wait before merging

@abretaud
Copy link
Contributor Author

ping @bgruening as we discussed: using maker in mpi mode would probably look like this
(but this pr is not yet ready I think)

@bgruening
Copy link
Member

Thanks for sharing @abretaud! Have a save trip home!

@abretaud
Copy link
Contributor Author

Hum, the error looks like bioconda/bioconda-recipes#13562, I'll have a look next week

@abretaud
Copy link
Contributor Author

abretaud commented May 3, 2019

Green!
I think it's ready to merge now. To sum up, it uses mpich now to run maker in mpi mode:

  • if it runs on a destination which supports mpi, fantastic, it can use thousands of cpus on hundreds of compute nodes. e.g. running this on a slurm queue with option -n
  • if the destination doesn't support mpi, it works too, it will be able to use GALAXY_SLOTS number of cpus on the compute node (or even a single one if there is only one available)

ping @erasche @shiltemann with this PR the maker tutorial should run much faster, so it could be useful for the Utrecht session. Do you think it would be possible to try it, or do you prefer to keep the current non-mpi version?

I can open a Galaxy issue to start a discussion on a better way to standardize mpi execution

@hexylena
Copy link
Member

hexylena commented May 3, 2019

@abretaud of course we want a faster version! :)

@abretaud
Copy link
Contributor Author

abretaud commented May 3, 2019

@erasche cool!
I opened the galaxy issue: galaxyproject/galaxy#7884

@bgruening
Copy link
Member

Thanks for creating the issue and the update.

@bgruening bgruening merged commit 4141a68 into galaxyproject:master May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants