OSC / ood_core

Open OnDemand core library
https://osc.github.io/ood_core/
MIT License
10 stars 28 forks source link

Slurm gpus_from_gres not quite right #755

Closed johrstrom closed 2 years ago

johrstrom commented 2 years ago

If that is kept, I think that regex is wrong. Where it might cause issues:

gres/gpu:v100-32g=2

That's a GRES definition on Pitzer, numbers are in the subtype. Depending on where you query the GRES, ie which command, you could also split on = and get last element.

_Originally posted by treydock in https://github.com/OSC/ood_core/pull/752#discussion_r845437518_

┆Issue is synchronized with this Asana task by Unito

johrstrom commented 2 years ago

This is the method that this ticket is about: https://github.com/OSC/ood_core/blob/b31144793cb243e5af7785ed1e07023a8a131037/lib/ood_core/job/adapters/slurm.rb#L620-L622

Looks like /gpu:[^,]*(\d+),{,1}/ could work. @treydock seems like we're only interested in the very last digit(s) whatever that maybe.

We could split(',') and capture from the end of the line $ (positive lookbehind isn't implemented Ruby) but I don't know if I like chaining a splits together.

lukew3 commented 2 years ago

I'm pretty sure that the regex still works. Check out the tests I added in #754. The regex is grabbing the last set of digits after gpu: and before , or the end of the string.

johrstrom commented 2 years ago

Yep, I added similar test cases, but didn't check to see if they worked. In any case, yes with new tests there's nothing to do.