search

qiime2 / galaxy-tools

Official QIIME 2 tools for Galaxy
BSD 3-Clause "New" or "Revised" License
1 stars 4 forks source link

Filter out parallel parameters #33

Closed ebolyen closed 5 months ago

ebolyen commented 1 year ago

As indicated by @bernt-matthias in #32, Galaxy tools should not provide the user with parameters to control the underlying resource usage. Currently parameters such as n_jobs/n_threads/n_cores/etc are all basic Integer or String types, which means that we would have to rely on the parameter name to "guess" at the purpose.

Ideally we update the Framework to have a Parsl-compatible notion of the above concepts as primitive types, which we can then filter out more intentionally. @Oddant1, would you be able to do some investigative work to figure out what is going to be the most compatible option here? We don't want these new primitive types to show up to the user in Galaxy (unlike other interfaces), but we may be able to use them to template out some basic job configs for administrator convenience.

Once the framework is updated, we will need to update q2galaxy, all other interfaces to recognize the new primitive, and any plugins using these resource parameters.

Additional docs: https://docs.galaxyproject.org/en/latest/admin/jobs.html#dynamic-destination-mapping

Oddant1 commented 11 months ago

@ebolyen these parameters are currently split between Int % Range(1, None) | Str % Choices(['auto']) and Int % Range(0, None) with 0 as auto. It would be nice to standardize these type and these names (although they are possibly inspired by the names/values of the params they wrap in the tools we are wrapping), but those would obviously create breaking changes in a good few places. We can probably just wrap both of those types that are currently in use.

bernt-matthias commented 7 months ago

This would be really cool to get it fixed. The number of cores/threads needs to be set to $GALAXY_SLOTS which will be the number of slots that can be used by the job. Galaxy / the Galaxy admin sets this automatically.

If users set this manually the jobs will be killed on most HPC systems.

gregcaporaso commented 7 months ago

Thanks for bumping this up @bernt-matthias. We'll discuss this today and follow-up with a plan.

gregcaporaso commented 7 months ago

Hi @bernt-matthias, This change is going to require some backward incompatible interface changes, which we warn our users about one release cycle before they go live. So our plan is to warn our users with the 2023.12 release (scheduled for 11 Dec 2023), and then make the changes for our first 2024 release, which I expect will be in February.

bernt-matthias commented 7 months ago

Great news! Thanks for keeping me posted. Looking forwards to the next release :)

colinvwood commented 5 months ago

implemented in qiime2/q2galaxy#47