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

Update docker build #16

Merged
merged 6 commits into from
May 28, 2024
Merged

Update docker build #16

merged 6 commits into from
May 28, 2024

Conversation

StephenOman
Copy link
Collaborator

This PR updates the Docker Distribution capability

  • Removes old Ubuntu distros
  • Add Ubuntu 22.04 (Jammy)
  • Update build for multi-architecture deployment (x86_64, amd64 and arm64)
  • Changed README instructions

Copy link
Owner

@heiner heiner left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

Remind me what I need to do re: automatic building of these images?

./miniconda.sh -b -p /opt/conda && \
/opt/conda/bin/conda install -y python=$PYTHON_VERSION && \
/opt/conda/bin/conda clean -ya
RUN case ${TARGETPLATFORM} in \
Copy link
Owner

Choose a reason for hiding this comment

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

These days, I'd probably use uv and nothing else but we don't need to change this in this PR.

Copy link
Owner

Choose a reason for hiding this comment

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

That would probably help save GitHub minutes too as the non-nethack parts are likely the majority of the build time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What's uv?

Copy link
Owner

Choose a reason for hiding this comment

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

https://astral.sh/blog/uv

I'm already using it in one of the tests, here.

It's from the ruff developer and one of the many Rust-for-Python tools that massively speed up things these days.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On my M3 Mac, the Docker build takes 15 minutes and the image size is a chunky 23GB.

Copy link
Owner

Choose a reason for hiding this comment

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

Is it faster the second time and we just have to cache the Docker layers?

Alternatively, I think the base image with CUDA is quite heavy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is faster, but still takes nearly 10 minutes, most of the time is in RUN pip install '.[all]' on amd64 (~400s). Rather strangely, the same step for arm64 only takes about 40s. I wonder is this due to me running the build on a Mac.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that's likely. Cross compilation is rather slow on arm MacBooks.

@StephenOman
Copy link
Collaborator Author

Remind me what I need to do re: automatic building of these images?

Depends on which solution we want to pursue, Docker Hub or GitHub Container Registry. For now I suggest letting users build their own Docker images per instructions and look to automating the image building to coincide with NLE-1.0.0.

@StephenOman StephenOman merged commit e3e4253 into main May 28, 2024
15 checks passed
@StephenOman StephenOman deleted the update-docker-build branch May 28, 2024 07:39
@StephenOman StephenOman added the maintenance General maintenance and version updates label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance General maintenance and version updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants