Skip to content

Fixed Dockerfile for Apple Silicon (M4 Pro)#35

Merged
alfieadhemar merged 4 commits intomasterfrom
dev-vera-br-issue34
Nov 20, 2025
Merged

Fixed Dockerfile for Apple Silicon (M4 Pro)#35
alfieadhemar merged 4 commits intomasterfrom
dev-vera-br-issue34

Conversation

@vera-br
Copy link
Collaborator

@vera-br vera-br commented Nov 6, 2025

Fixes issue #34

Changes:

  • Set ARM64-specific optimisation flags
  • Implemented a shallow clone of HDF5 to reduce data transfer and avoid fetch-pack disconnects during the build process

Testing:

  • Successfully built and tested on Apple M4 Pro (macOS 26.1)
  • All unit tests passed for ReMKiT1D v1.3.0

@vera-br vera-br added the bug Something isn't working label Nov 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the ReMKiT1D Docker container for ARM64/Apple Silicon architecture. The changes include version bump to 1.3.0, removal of Python dependencies that weren't needed, ARM64-specific compiler optimizations, improved reliability for HDF5 installation, and environment variable cleanup.

Key Changes:

  • Added ARM64-specific optimization flags (-mcpu=native) to PETSc configuration
  • Implemented fallback mechanism for HDF5 installation to handle git clone failures
  • Removed duplicate LD_LIBRARY_PATH environment variable declarations in favor of RUN echo commands

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 129 to +147
RUN echo LD_LIBRARY_PATH=/home/mpich-install/lib:$LD_LIBRARY_PATH
RUN echo LD_LIBRARY_PATH=/home/sundials/lib:$LD_LIBRARY_PATH
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

These RUN echo commands only print the variable values but don't actually set them. The LD_LIBRARY_PATH environment variable is not being configured. Either remove these lines if they're debugging statements, or replace them with ENV LD_LIBRARY_PATH=/home/mpich-install/lib:/home/sundials/lib:$LD_LIBRARY_PATH to properly set the variable.

Suggested change
RUN echo LD_LIBRARY_PATH=/home/mpich-install/lib:$LD_LIBRARY_PATH
RUN echo LD_LIBRARY_PATH=/home/sundials/lib:$LD_LIBRARY_PATH
ENV LD_LIBRARY_PATH=/home/mpich-install/lib:/home/sundials/lib:$LD_LIBRARY_PATH

Copilot uses AI. Check for mistakes.
ENV PATH=$PATH:/home/installs/petsc
ENV PATH=$PATH:/home/installs/hdf5
ENV PATH=$PATH:/home/installs/json-fortran/jsonfortran-gnu-8.2.5
ENV PATH=$PATH:/home/mpich-install/bin:$PATH
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The PATH is appended with :$PATH at the end, which creates a redundant reference. This line adds /home/mpich-install/bin to PATH and then appends the entire PATH again. It should be ENV PATH=$PATH:/home/mpich-install/bin or ENV PATH=/home/mpich-install/bin:$PATH.

Suggested change
ENV PATH=$PATH:/home/mpich-install/bin:$PATH
ENV PATH=$PATH:/home/mpich-install/bin

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.20%. Comparing base (3b0aa31) to head (b9645c4).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   54.20%   54.20%           
=======================================
  Files         239      239           
  Lines       10124    10124           
=======================================
  Hits         5487     5487           
  Misses       4637     4637           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

alfieadhemar and others added 3 commits November 7, 2025 10:06
Cleaning whitespace

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Cleaning whitespace

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Cleaning whitespace

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Collaborator

@alfieadhemar alfieadhemar left a comment

Choose a reason for hiding this comment

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

Dockerfile succsefully compiled/verified on M2

@alfieadhemar alfieadhemar merged commit abc9606 into master Nov 20, 2025
4 checks passed
@SMijin SMijin deleted the dev-vera-br-issue34 branch November 27, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants