-
Notifications
You must be signed in to change notification settings - Fork 0
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
Updates aide-svrtk MAP with MONAI-based CNN #13
base: main
Are you sure you want to change the base?
Conversation
TODO:
|
the paths to MIRTK were updated to /bin - the docker script is now copied from https://github.com/SVRTK/auto-proc-svrtk |
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.
@alenauus – the MAP is not quite working yet. See line-by-line comments below for some initial suggestions.
To test your changes, I recommend the following process:
- Change line 1 in
app/Dockerfile
to create new version (0.2.1) of the MAP so you don't conflict with the released version (0.2.0), e.g.:
FROM ghcr.io/svrtk/aide-svrtk/map-init:0.2.1 AS build
-
Make your changes ...
-
Build the first stage of the MAP (known as "map-init" – this is a workaround because we need to put SVRTK into the MAP, which is third-party non-Python code):
monai-deploy package app -t ghcr.io/svrtk/aide-svrtk/map-init:0.2.1 -r requirements.txt -l DEBUG
- Build the final MAP:
docker build -t ghcr.io/svrtk/aide-svrtk/map:0.2.1 app/
-
Create
input
andoutput
directories and put some 2D DICOM stacks in the input folder -
Run the MAP to test your changes:
monai-deploy run ghcr.io/svrtk/aide-svrtk/map:0.2.1 input/ output/
Everytime you make a change, repeat steps 3–6.
I expect it will error, but hopefully you should see it at least make some progress. Give that a go and then we can look at the next bits to fix.
|
||
WORKDIR /home | ||
|
||
# Setup 3D UNet models | ||
RUN git clone https://github.com/SVRTK/Segmentation_FetalMRI.git --branch svrtk-docker-gpu-0.10 --single-branch /home/Segmentation_FetalMRI | ||
RUN git clone https://github.com/SVRTK/auto-proc-svrtk.git /home/auto-proc-svrtk |
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.
Most of the errors are to do with this line currently. You've changed the location of the recon Shell script, but not updated the rest of the Dockerfile and Python code to reflect the new script location, so... see other comments where I can see you need to make changes.
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.
i fixed paths and was able to run monai-deploy run ghcr.io/svrtk/aide-svrtk/map:0.2.1 input/ output/
app/Dockerfile
Outdated
|
||
# Bugfix: without below, cannot import torch within Python | ||
# Error: OSError: /opt/hpcx/ompi/lib/libmpi.so.40: undefined symbol: opal_hwloc201_hwloc_get_type_depth | ||
# Fix: https://forums.developer.nvidia.com/t/issues-building-docker-image-from-ngc-container-nvcr-io-nvidia-pytorch-22-py3/209034/5 | ||
ENV PATH="${PATH}:/opt/hpcx/ompi/bin" | ||
ENV LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/opt/hpcx/ompi/lib" | ||
|
||
COPY docker-recon-brain-auto.bash /home/scripts/docker-recon-brain-auto.bash | ||
COPY /home/auto-proc-svrtk/scripts/auto-brain-reconstruction-aide.sh /home/scripts/auto-brain-reconstruction.sh |
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.
You can remove this line entirely. The COPY
command in Docker copies from your local machine into the corresponding place in the container. It fails here because /home/auto-proc-svrtk/scripts/auto-brain-reconstruction-aide.sh
doesn't exist in the aide-svrtk
repo.
Instead, you clone the auto-proc-svrtk
repo into the container further up this file, which means auto-brain-reconstruction.sh
will be downloaded into the container already.
One thing I noticed – you have two scripts in here, one of which is labelled "aide". Make sure you are pointing at the correct script.
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.
updated thank you
app/Dockerfile
Outdated
RUN mkdir -p /home/recon \ | ||
&& mkdir -p /home/output \ | ||
&& chmod +x /home/scripts/* |
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.
The most important thing here is the chmod
on the scripts folder. This enables the Docker container to run the shell scripts in the folder. You need to update this so that it points at the correct folder in the cloned auto-proc-svrtk
repo.
I think you can remove the mkdir
lines – I don't think they are needed anymore.
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.
updated thank you
# if torch.cuda.is_available(): | ||
# cnn_mode = "1" | ||
# logging.info("SVRTK reconstruction using GPU mode ...") | ||
# if not torch.cuda.is_available(): | ||
# cnn_mode = "-1" | ||
# logging.info("SVRTK reconstruction using CPU mode ...") | ||
# | ||
# motion_correction_mode = "-1" # -1 minor, 1 severe | ||
# logging.info("SVRTK reconstruction ...") |
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.
I'm not keen on commenting this out. I see you've shifted the cnn_mode/motion_correction_mode variables into the bash script. The problem is that it puts a hard requirement on GPU. In testing, I've found it useful to run in CPU mode.
Leave it for now, but something we can discuss.
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.
the current recon code is fully CPU based (it is fast)
# logging.info("SVRTK reconstruction using CPU mode ...") | ||
# | ||
# motion_correction_mode = "-1" # -1 minor, 1 severe | ||
# logging.info("SVRTK reconstruction ...") | ||
|
||
subprocess.run([ | ||
"/home/scripts/docker-recon-brain-auto.bash", |
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.
This line refers to the old recon .bash
script. It needs to be updated to reflect the new shell script at the correct location in the container.
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.
updated, thank you
updated paths in the docker file and recon operator & references to auto-svrtk-proc repo |
Resolves #10