-
Notifications
You must be signed in to change notification settings - Fork 144
Create a docker multistage build script #215
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
Conversation
|
@AhmedFaisal95 Please have a look and see what still needs to be done. At a minimum, this drastically reduces the doubling of code. It seems that DOCKER_BUILDKIT also supports the script now. Which speeds up builds a lot, since it builds stages that are independent in parallel. For example |
52b0abf to
f4f108d
Compare
|
This seems to run fine except when:
Case 1Command: Result: Case 2Command: Result: |
|
The issues mentioned above should be fixed in this PR: dkuegler#2 |
e21db27 to
a76d4c7
Compare
|
@AhmedFaisal95 Can you confirm this works now? We need a "basic test" of all images :). |
|
@m-reuter The Apple M1 device is missing from the docker multistage built. The reason is that it was not in For the AMD build, the base image got updated and I am not really sure if the updated image works. Maybe, someone can check this as well? |
I already tested the fixes before submitting the PR, specifically all images except for the AMD case (no hardware). |
9024e01 to
d187232
Compare
d187232 to
5007bd9
Compare
|
The build script should also compile the byte code https://docs.python.org/3/library/py_compile.html . |
5007bd9 to
e11150d
Compare
|
I think what is left for this PR to be merged is proper testing. |
For M1 we do not use a docker, only native install on Mac. Inside Docker it was much slower for yet unknown reasons, see #109 |
e11150d to
3660c30
Compare
|
@agirodi can you test the AMD docker script? |
3660c30 to
0310bbe
Compare
7e23aea to
160d867
Compare
I just did and it is working correctly. The build went without issues and running produced a correct segmentation. |
|
Then, this PR should be ready for merging. |
160d867 to
53d6070
Compare
53d6070 to
df70ab0
Compare
|
A few points for documentation:
The advantage of this approach is, that for future docker builds we can pull the base release images and use them (in most cases they probably exist locally anyway). Adding FastSurfer is then extremely fast :-) |
df70ab0 to
458828d
Compare
1cdab68 to
02a0b3f
Compare
02a0b3f to
a868f83
Compare
9813c67 to
ca50067
Compare
4f689e7 to
5801fed
Compare
…urfer interface rework. All images are built with the "core" Dockerfile at Docker/Dockerfile. Also update some additional folders to not be copied automatically by the build (.dockerignore).
This enables building runtime_surf_only with DEVICE=none. Fix: Change build_conda_cpu base (build_conda_base --> build_conda_common)
Add some more files and folders to .dockerignore Remove the chmod on checkpoints command from the Dockerfile
change the file with BUILD information to BUILD.txt (instead of VERSION.txt) change the formatting options in segstats.py
Fundamentally, the cpu image now is ~3GB and does not include the cudatoolkit any more. pytorch is now installed from the pytorch download page with all relevant platform-specific requirements such as the cudatoolkit or rocm drivers/runtime. All this now always part of the conda environment. - Update/fix build.py build script for python - add conda-pack script to pack the conda environment - update the Docker/README.md - added an automatically created build.log file into each subject directory - added a BUILD.info file into the docker container, that contains version information about version number, branch, git status at build time, checkpoints contained in container, and pip package list. - added a FastSurferCNN/version.py script that replaces the version-related commands in run_fastsurfer.sh - added baked in support for AWS - remove torchio as standard imports in FastSurferCNN/data_loader/(dataset|loader).py, so we do not need torchio at inference time any more. - added FastSurferCNN/utils/run_tools, a wrapper to subprocess.Popen that allows easier, non-blocking executiong of scripts and binaries. - added BUILD.info to .gitignore - removed BUILD.txt from .dockerignore - run_fastsurfer.sh now also supports a --version command, see --help - deactivate user-site imports (-s flag to python)
- improve debug info of image (in /install folder, if --build-arg DEBUG=true) - add --progress=plain to docker build if running inside the python script because no over-write on the console is possible - merge different pip sections in the Dockerfile, as multiple sections were not merged automatically by conda - filter __pycache__ from version - up ubuntu default versions to 22.04 - fix python -s for no user directory packages
- add `--tag my_fastsurfer:<device>` in README.md - remove rocm environment variable - add documentation for HSA_OVERRIDE_GFX_VERSION
5801fed to
b296711
Compare
Create a docker multistage build script that is compatible with FastSurfer interface rework.
All images are built with the "core" Dockerfile at Docker/Dockerfile. Also update some additional folders to not be copied automatically by the build (.dockerignore).
TODOs: