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

Move QE conda install into separate stage #783

Merged
merged 3 commits into from
Jul 23, 2024
Merged

Move QE conda install into separate stage #783

merged 3 commits into from
Jul 23, 2024

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented Jul 22, 2024

When building the QEApp Docker container, the Quantum Espresso installation into a conda environment is a step that's completely independent from the others. By putting it into a separate Docker build stage, it can be build in parallel (when using buildkit), as:

DOCKER_BUILDKIT=1  docker build .

Also added a .dockerignore file. This is important since we're copying the whole directory into the container and we don't want to copy junk inside. Even if we later run git clean, once the files are copied, that stay in the fs layers.

Extracted from #781

When building the QEApp Docker container,
the Quantum Espresso installation into a conda environment
is a step that's completely independent from the others.
By putting it into a separate Docker build stage,
it can be build in parallel (when using buildkit), as:

    DOCKER_BUILDKIT=1  docker build .
@danielhollas danielhollas requested a review from unkcpz July 22, 2024 21:04
# Install QE into conda environment in /opt/conda
# This step is independent from the others and can be run in parallel.
FROM ghcr.io/aiidalab/full-stack:${FULL_STACK_VER} AS qe_conda_env
ARG QE_VER
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed so that the variables are passed from the top level.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, learned. I was always defined in the stage and don't know how to move all ARGs to the front of file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's definitely not obvious, took me a while to figure out.

uv pip install --system --no-cache . && \
fix-permissions "${CONDA_DIR}" && \
fix-permissions "/home/${NB_USER}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not needed since the uv pip install --system does not touch the home directory.

@danielhollas danielhollas requested a review from superstar54 July 22, 2024 21:07
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.28%. Comparing base (122efe1) to head (d2a75e4).
Report is 36 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #783   +/-   ##
=======================================
  Coverage   68.28%   68.28%           
=======================================
  Files          45       45           
  Lines        4143     4143           
=======================================
  Hits         2829     2829           
  Misses       1314     1314           
Flag Coverage Δ
python-3.10 68.28% <ø> (ø)
python-3.9 68.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks!

# Install QE into conda environment in /opt/conda
# This step is independent from the others and can be run in parallel.
FROM ghcr.io/aiidalab/full-stack:${FULL_STACK_VER} AS qe_conda_env
ARG QE_VER
Copy link
Member

Choose a reason for hiding this comment

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

Cool, learned. I was always defined in the stage and don't know how to move all ARGs to the front of file.

@danielhollas danielhollas merged commit a3b40b0 into main Jul 23, 2024
12 checks passed
@danielhollas danielhollas deleted the qe-stage branch July 23, 2024 12:17
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.

2 participants