Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Adds support for submitting jobs with gpus #794

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

dposada
Copy link
Contributor

@dposada dposada commented Mar 29, 2018

Changes proposed in this PR

  • adding --gpus flag to cs submit
  • adding an integration test for --gpus

Why are we making these changes?

This allows users to submit jobs with gpus using cs. See issue #793 for more context.

@dposada
Copy link
Contributor Author

dposada commented Mar 29, 2018

@dPeS - Any chance you can take this for a spin and see if it works in your environment?

@dPeS
Copy link
Contributor

dPeS commented Mar 31, 2018

@dposada sure, will do next week and report here when done

@dPeS
Copy link
Contributor

dPeS commented Apr 5, 2018

@dposada works as expected... but I see 1 issue here:

cs submit --gpus 1 sleep 1 works ok now, but:

cs submit sleep 1 waits infinitely since there is no host without GPU

is it expected?

@dposada
Copy link
Contributor Author

dposada commented Apr 5, 2018

Currently, non-gpu jobs won't get scheduled on agents with gpus (@pschorf correct me if I'm wrong)

@dposada dposada removed the wip label Apr 5, 2018
@dposada dposada requested a review from DaoWen April 5, 2018 17:19
@pschorf
Copy link
Contributor

pschorf commented Apr 5, 2018

@dPeS That's correct. Currently, we have logic to prevent scheduling jobs without GPUs on GPU-enabled hosts to prevent tying up the non-GPU resources on the host. If that's an issue in your environment we can look into making that configurable.

@pschorf
Copy link
Contributor

pschorf commented Apr 5, 2018

@DaoWen any feedback before I merge this?

@DaoWen DaoWen merged commit 39e0cae into twosigma:master Apr 5, 2018
@dposada dposada deleted the cs-submit-gpus branch April 5, 2018 20:40
@dPeS
Copy link
Contributor

dPeS commented Apr 6, 2018

@pschorf hi, that would be useful for my case... it's not a bloker since I could spawn p2.xlarge + some non gpu instance, but in future configuration option will be cool

@dposada
Copy link
Contributor Author

dposada commented Apr 6, 2018

I created #803 to track this.

Thanks for testing this PR, @dPeS!

@dposada dposada mentioned this pull request Apr 6, 2018
dposada added a commit to dposada/Cook that referenced this pull request Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants