Skip to content
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

add Dockerfile (#1) #3

Merged
merged 3 commits into from
Jun 7, 2020
Merged

add Dockerfile (#1) #3

merged 3 commits into from
Jun 7, 2020

Conversation

nicolecastillo
Copy link
Owner

  • This commit adds the Dockerfile that assembles the image that will be used for the Popper workflow file.

This commit adds the Dockerfile that assembles the image that will be used for the Popper workflow file.
@nicolecastillo nicolecastillo changed the title add Dockerfile add Dockerfile (#1) Jun 5, 2020
Copy link
Collaborator

@ivotron ivotron left a comment

Choose a reason for hiding this comment

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

this is looking great! let me know if anything of the feedback I left doesn't make sense and I can clarify. thanks!

@@ -0,0 +1,22 @@
FROM python:3.6

ENV USER=root
Copy link
Collaborator

Choose a reason for hiding this comment

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

root is already the user for base images, so we can remove this line

gcc \
binutils \
clang \
cmake
Copy link
Collaborator

Choose a reason for hiding this comment

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

gcc is a dependency of cmake, so looks like the minimal requirements are cmake and g++, the rest we can remove

# Install liblinear
RUN cd ..
RUN git clone https://github.com/cjlin1/liblinear.git
RUN cd liblinear/python/ && make
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of installing from repos, we can install with pip:

pip install xlearn==0.40a1 liblinear==2.11.2

@@ -0,0 +1,22 @@
FROM python:3.6
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can use 3.7 as that's the latest stable release

RUN cd liblinear/python/ && make


ENTRYPOINT ["/bin/bash"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can also reduce the size of the image by making use of multi-stage builds, as it would allow us to not have to include the compiler and all the auxiliary files that get created when the libraries get installed.

For this we need to add a name to the first image:

FROM python:3.7-slim-buster AS base

RUN ...
# all the commands you have above that install the libraries, then:


FROM python:3.7-slim-buster AS lib
COPY --from base /usr/local/lib/python3.7/site-packages /usr/local/lib/python3.7/

the last two lines say something like: "create a new layer from the python:3.7-slim-buster image, and in that image copy from the base image the site-packages folder".

FROM python:3.7-slim-buster AS lib
COPY --from=base /usr/local/lib/python3.7/site-packages /usr/local/lib/python3.7/

ENV USER=root
Copy link
Owner Author

Choose a reason for hiding this comment

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

@ivotron When I remove the root line I'm not able to run the tests from the demo, do you know why this happens?

Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure. can you paste the commands you're using to run the demo so I can reproduce the issue on my end?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sure. So once I run the image this is what I do:

  • cd xlearn/demo/classification/higgs/
  • python run_higgs.py

Copy link
Collaborator

Choose a reason for hiding this comment

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

do you that inside a container? or as part of building the dockerfile? I'll ask on slack

Copy link
Collaborator

@ivotron ivotron Jun 7, 2020

Choose a reason for hiding this comment

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

it looks like the library assumes the USER variable exists, otherwise fails: https://github.com/aksnzhy/xlearn/blob/43fd6819d5b514099db7b85b27ebd010e0ea230e/src/base/system.h#L48 . This is a bit unorthodox but that's what it does, so we need to define it as you had it.

Also, I hadn't realized that the package on pypi is quite old. So we will have to build from source as you were doing before, sorry! This is what it ended up working on my end:

FROM python:3.7-slim-buster as base

RUN apt update && \
    apt install -y cmake g++ && \
    rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

COPY . /xlearn

# build from source (installs to /usr/local/lib/python3.7/site-packages/)
RUN cd /xlearn && \
    ls -l && \
    ./build.sh && \
    rm -r /xlearn

# install other library (installs to same site-packages path)
RUN pip install --no-cache-dir liblinear==2.11.2

# create an image without build dependencies
FROM python:3.7-slim-buster AS lib
ENV USER=root
COPY --from=base /usr/local/lib/python3.7/site-packages/* /usr/local/lib/python3.7/

I built the above with:

cd xlearn/
docker build -t xlearn -f demo/docker/Dockerfile .

And then ran it with:

docker run --rm \
  --volume $PWD:/workspace \
  --workdir=/workspace/demo/classification/higgs/ \
  xlearn \
    python run_higgs.py

Copy link
Owner Author

Choose a reason for hiding this comment

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

Awesome, thanks!

@ivotron ivotron merged commit a0bee46 into popper Jun 7, 2020
@ivotron ivotron deleted the add-dockerfile-pull-request branch June 7, 2020 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants