Skip to content

Add build matrix for docker images, update images #1936

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.

Sign up for GitHub

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

Jump to bottom
Merged
MMathisLab merged 16 commits into from
Sep 17, 2023
Merged

Add build matrix for docker images, update images #1936

MMathisLab merged 16 commits into from
Sep 17, 2023

Conversation

@stes
Copy link
Collaborator

@stes stes commented Aug 5, 2022 •
edited
Loading

  • Improvements to the build process for docker images: With the specified build matrix, it is now easier to build images with different DeepLabCut and CUDA base container versions
  • Support two version of deeplabcut, as well as CUDA 11.4 (as before) and the more recent CUDA 11.7

Open:

  • Build containers, and attach the full build log
  • Update containers on DockerHub
  • Discuss addition of additional CUDA versions/base images
  • Update PyPI package to interface with the containers

Build and test logs

https://gist.github.com/stes/80412bac41a37cabb6248db5e5d2884e

Testing

If you come across this PR and want to test it, please see this discussion for instructions (until this package is updated):

# make executable
chmod +x ./deeplabcut_docker.sh
# pull the image
docker pull deeplabcut/deeplabcut:2.2.1.1-gui-cuda11.7.0-runtime-ubuntu20.04
# run the UI
./deeplabcut_docker.sh gui

This should open the deeplabcut UI. If you are on a linux machine, you might run xhost + before executing the last command if you get a permission error.

Fixes #2164

@natsukium natsukium mentioned this pull request Feb 28, 2023
Closed
2 tasks
natsukium
natsukium reviewed Feb 28, 2023
View reviewed changes
docker/build.sh Outdated
2.2.1.1
do
for stage in base core test gui jupyter; do
tag=${deeplabcut_version}-${stage}-cuda${cuda_version}
Copy link

@natsukium natsukium Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tag also needs to be changed.

@MMathisLab
Copy link
Member

MMathisLab commented Mar 3, 2023

@stes @jeylau let's finish and merge

@MMathisLab MMathisLab requested a review from jeylau March 3, 2023 13:54
@MMathisLab MMathisLab marked this pull request as ready for review March 3, 2023 13:55
@MMathisLab
Copy link
Member

MMathisLab commented Mar 18, 2023

@jeylau can we fix this? // get it merged DLC broader docker support is quite important for a lot of applications, including in our lab ;)

jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
jeylau
jeylau reviewed Mar 28, 2023
View reviewed changes
@jeylau
Copy link
Contributor

jeylau commented Mar 28, 2023

"core" now works; I will test the GUI tomorrow on Ubuntu.

@maryapp
Copy link
Collaborator

maryapp commented May 13, 2023

@MMathisLab, can you please, merge this branch?

  • The matrix code is functional
  • the Dockerfile was adjusted by Jessy (it's minor changes dor cuda & cudnn, but in any cases ARG parameters come from matrix bash script, so it's mostly for coherency).

Once it merged, I will PR to:

  • : adjust docker developer-guide README
  • the change versions in Dockerfile & matrix that will correspond to images on dockerhub.
    (I can't PR beautifully now, as it's fork from external branch, and I would prefer to avoid new forks/PR on stes repo, to keep the history more linear)

@MMathisLab MMathisLab requested review from jeylau, maryapp and n-poulsen and removed request for jeylau May 16, 2023 10:53
@bagnall-lab bagnall-lab mentioned this pull request Jun 1, 2023
Closed
@MMathisLab
Copy link
Member

MMathisLab commented Jun 26, 2023

@maryapp can we please finalize this? cc @stes

@maryapp
Copy link
Collaborator

maryapp commented Aug 17, 2023

@MMathisLab Here is my PR to @stes PR! Hope this time it will make the affair! 🙌

@stes
Copy link
Collaborator Author

stes commented Aug 25, 2023

All right, some progress here!

Thanks a lot to @maryapp for taking over and adding support for the latest deeplabcut version / upgrading the images.

The changelog can be seen in my forked repo here: stes#3

If all tests pass (for docker --- I assume @maryapp checked these), this is good to go for merging into main from my end.

cc @MMathisLab @AlexEMG

@stes
Copy link
Collaborator Author

stes commented Aug 25, 2023

An open Q is maybe the faith of the deeplabcut-docker python package. It seems GUI support will be dropped with the latest version of this PR --- so I think we should decide whether to remove that, or use the legacy images in the pypi package. Opinions?

@maryapp
Copy link
Collaborator

maryapp commented Aug 30, 2023 •
edited
Loading

If all tests pass (for docker --- I assume @maryapp checked these), this is good to go for merging into main from my end.

Yes, all passed.

An open Q is maybe the faith of the deeplabcut-docker python package. It seems GUI support will be dropped with the latest version of this PR --- so I think we should decide whether to remove that, or use the legacy images in the pypi package. Opinions?

I would vote for the 1st option. I will check on the python package code (also how it picks up the right cuda version support, etc.)

@MMathisLab
Copy link
Member

MMathisLab commented Sep 13, 2023

use the legacy images in the pypi package. Opinions?

We need to keep the GUI support in legacy versions.

Please then decide if this can be merged ...

@maryapp
Copy link
Collaborator

maryapp commented Sep 13, 2023

@stes can you, please, elaborate what is "legacy" images for you? You are referencing the very first old images that are distributed via docker pip package now because of the tag?

@MMathisLab
Copy link
Member

MMathisLab commented Sep 17, 2023

I am merging as this is becoming unmanageable -- we need docker support...

@MMathisLab MMathisLab merged commit 4f5b00d into DeepLabCut:main Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maryapp maryapp Awaiting requested review from maryapp

3 more reviewers

@natsukium natsukium natsukium left review comments

@jeylau jeylau jeylau left review comments

@n-poulsen n-poulsen n-poulsen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@maryapp maryapp

@n-poulsen n-poulsen

Labels

Docker

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

container image has no cudnn

6 participants

@stes @MMathisLab @jeylau @maryapp @natsukium @n-poulsen