-
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
Update GPU dockerfile to fully support GPU workflow #26
Conversation
ENV DEBIAN_FRONTEND=noninteractive | ||
|
||
# FIXME: can remove git after switch back to released version of back-projection | ||
RUN apt-get update && apt-get install -y --no-install-recommends unzip vim curl git build-essential gfortran libfftw3-dev && \ |
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.
Do you need vim
?
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 is standard for our plugins (see the cookiecutter Dockerfile). It's a pretty minimal package but provides of a lot of benefits when debugging within the container.
|
||
# FIXME: can remove git after switch back to released version of back-projection | ||
RUN apt-get update && apt-get install -y --no-install-recommends unzip vim curl git build-essential gfortran libfftw3-dev && \ | ||
apt-get clean && rm -rf /var/lib/apt/lists/* |
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.
Why rm -rf /var/lib/apt/lists/*
?
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 removes unneeded apt
metadata that serves no purpose other than bloating the container.
# MULTIARCH_DIR=/usr/lib/$(gcc -print-multiarch) | ||
# FFTW_LIB=$MULTIARCH_DIR/libfftw3f.a |
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.
Are the commented-out lines still needed?
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.
Yeah, we may need to revert to this at some point, so I'd rather keep these close at hand.
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.
LGTM, feel free to merge unless you think any of my comments need to be addressed.
Dockerfile.gpu
so that outputs will contain actual data.scripts/build_proc.sh
to combine GPU compilation steps.scripts/ubuntu_setup.sh
for setting up a GPU-based Ubuntu EC2 AMI.README.md
to reflect the above changes.