-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Conversation
As you have seen in the mentioned ticket we would like to have CPU and GPU separated configs but I think is not possible currently if you check microsoft/vscode-remote-release#2143 (comment) |
repository-containers/github.com/tensorflow/addons/.devcontainer/Dockerfile
Outdated
Show resolved
Hide resolved
We are waiting to check test and test discover to see if we could enable |
@Chuxel is there a way to differentiate between CPU and GPU |
@bhack I would use a build arg if you need to modify the contents of the Dockerfile and add a comment into devcontainer.json about switching between the two. |
@Chuxel need we to comment and uncomment also microsoft/vscode-remote-release#345 (comment) right? |
@bhack If there's arguments you need to pass in, yes. The .NET Core definition is an example of this being done - in this case for SSL support: https://github.com/microsoft/vscode-dev-containers/blob/master/containers/dotnetcore-3.1/.devcontainer/devcontainer.json |
repository-containers/github.com/tensorflow/addons/.devcontainer/devcontainer.json
Show resolved
Hide resolved
@Chuxel I have another question how we could automate this manual commenting/uncommenting activity when we let the user to use this for Github codespaces? |
VS Codespaces (and by extension Codespaces in GitHub) will be implementing the ability to iterate on a devcontainer.json and Dockerfile like the Remote - Containers extension over time. It's just not quite there yet. |
@Chuxel do you mean that they have in the roadmap multiple alternative devcontainer.json and Dockerfile support for the same repo? |
@bhack You'll be able to uncomment and just rebuild like you can with Remote - Containers. Step one is getting to feature parity. |
@Chuxel Thanks. Could we still host |
Yeah, currently Codespaces does not pull from this repo, but this is part of the work we'll collaborate with that team on as iterating on a container starts to be available. We'd want the same list of definitions from this repo there as well, so that would be the natural time to include it. |
Disable pytest integration until upstream bugs are resolved
@seanpmorgan I've added the build-arg |
@Chuxel as you see we need to comment and uncomment two sections to switch from CPU to GPU. It is not the best UX solution but I don't find anything better with the current features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small comments to consider. Otherwise looks good.
repository-containers/github.com/tensorflow/addons/.devcontainer/devcontainer.json
Outdated
Show resolved
Hide resolved
repository-containers/github.com/tensorflow/addons/.devcontainer/devcontainer.json
Outdated
Show resolved
Hide resolved
It is ok for me. @seanpmorgan when you can give a pass with the GPU options we could merge this. |
@seanpmorgan @bhack Let me know when you think this is to the point it should be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies this is good to merge. There is some additional work that needs to be done on the base image but that's not gating this PR.
@Chuxel any news on this specific point? Where can I track progress on this? Any ticket? |
@Chuxel gently ping |
Add support to Tensorflow addons:
See tensorflow/addons#1309 and tensorflow/addons#1888