chanzuckerberg / miniwdl

Workflow Description Language developer tools & local runner
MIT License
172 stars 53 forks source link

Roadmap for adding `runtime` `gpu` attribute and `hints`. #631

Open williamrowell opened 1 year ago

williamrowell commented 1 year ago

Are there any plans to implement the runtime gpu attribute or to add hints? I'm interested in extending miniwdl-slurm to specify slurm partitions and schedule GPUs, but I need to be able to access gpu and provide custom runtime attributes.

I've mocked up the changes to miniwdl-slurm below, before I learned about the plans to move custom runtime attributes to the hints dict.

https://github.com/williamrowell/miniwdl-slurm/commit/29fc361d169a07b672fad4461ffb3842fb6509a3

mlin commented 1 year ago

@williamrowell Eventually but not in the short term, so adding it to miniwdl-slurm sounds right to me. You will need to add processing for the new attributes in the override of process_runtime() as well as _slurm_invocation(). The former takes the WDL.Value evaluated from the runtime WDL expression, checks if it's the appropriate type and adds it into self.runtime_values (typically as an unwrapped Python value). The latter actually does whatever is supposed to happen as a result. See the base class implementation of process_runtime() (which does at least load the gpu boolean mentioned in the WDL 1.1 spec, but that's all it does).

williamrowell commented 1 year ago

Thanks for the pointers. I've incorporated them here: https://github.com/williamrowell/miniwdl-slurm/commit/8cb4a4308a801ccd0fe70345be644c86c8469bc7

slurm_partition, slurm_constraint, and num_gpu are all working correctly for me at the moment. Is this something you'd be interested in accepting as a PR on miniwdl-slurm?

I'm still getting ignored runtime.gpu (not yet implemented) in my logs. Is it possible to override the logger message as well?

mlin commented 1 year ago

@williamrowell Cool, thanks for the update!

Is this something you'd be interested in accepting as a PR on miniwdl-slurm?

Let me ping @rhpvorderman @DavyCats here who led its development. From my perspective, it makes sense as long as it's reasonably likely to help other slurm users, but I don't have enough experience with slurm (and particularly how GPUs are provisioned with it) to form an opinion personally.

I'm still getting ignored runtime.gpu (not yet implemented) in my logs. Is it possible to override the logger message as well?

This warning is currently hard-coded in SlurmSingularity's 'grandparent' class which didn't anticipate eventually being used in this way. I'll try to move it to a more intelligent/appropriate place in the next miniwdl point release.

doron-st commented 2 months ago

Hi!

Any update regarding gpu support? I'm trying to run miniwdl locally for debugging purposes, and my workflow requires GPU. It would be great to have GPU support at least for this cases which may be simple.

Best regards, Doron