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

added a config option for easy gpu sharing to DockerSpawner #473

Closed
wants to merge 4 commits into from

Conversation

indirected
Copy link

@indirected indirected commented Dec 29, 2022

I added a config option: gpu_ids to make sharing GPUs with containers a little bit easier and tried to keep the style close to cpu_limit and mem_limit.
Also, How it works is documented with the help parameter.

Edit: I'm kind of new to the open-source community, sorry if something is missing. comments are appreciated!

@welcome
Copy link

welcome bot commented Dec 29, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@minrk
Copy link
Member

minrk commented Sep 18, 2023

Thanks for the PR and sorry for the delays! Trying to get this repo back on track. I think in general, we do not want to add new DockerSpawner-specific ways to express generic docker options that already work with the pass-through extra_host_config. Since I think requesting GPUs already works there, we shouldn't add gpu-specific options and instead let folks write the DeviceRequests directly. That will more appropriately direct people to the docker-py docs, rather than our own, which a custom option would do. The bit I see missing is that extra_host_config can't be a callable, unlike the new option here. I think the fix here should be to allow extra_host_config itself to be a callable, like most other options that may vary per-user. What do you think?

@indirected
Copy link
Author

Hi. Sorry for the late response. I am having a bit of academic pressure on my back right now.
I think your argument is valid and reasonable. The only thing that I am thinking is both cpu and memory limits are also docker arguments. That is the reason I wanted to add the gpu as an additional separate option. Since it is almost a general purpose device these days and people who run jupyter servers (very much with jupyter hub and docker spawner), just like me, would appreciate a handy option to specify gpu directly as an option of docker spawner, just like memory and cpu.

One more thing is, at least at the time that I needed to add gpu, the device request part of the docker-py docs, was not complete and not helpful at all.

Again, I understand your argument and believe it is valid. I am just trying to bring ideas that how this option would benefit some people. Thanks!

@minrk
Copy link
Member

minrk commented Oct 5, 2023

Thanks for your input! What distinguishes cpu and memory limits is that they are standardized by jupyterhub, so all Spawners have the same configuration for implementing a consistent resource limit. There is not an equivalent for GPUs.

If it is possible with the passthroughs we have, perhaps the right thing to add is an example showing how to do it with extra_host_config.

@indirected
Copy link
Author

Hmmm. Yes I think it is best to keep it as is but include an example. Thank you!

@minrk minrk closed this in #500 Nov 14, 2023
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.

None yet

2 participants