Add cmake preset to compile 4C on Darwin - #1795
Conversation
8835283 to
a780d98
Compare
|
I have tested the preset on a clean system. For the dependencies there are few remarks:
Lastly, the tools required to compile on the Mac is at least: |
138a762 to
280ccbd
Compare
Are there any issues when using CMake 4.x.x? |
|
What are the future plans for adding all these Darwin-specific install scripts? Do you want to set up a docker build? These files should be tested in some way or they will stop working quickly. It would be OK to do this in a second step, I think. I am not thrilled to see a new CMake preset, as I believe we should rather work to remove more and more of these. They are specific to your system, and the LNM and IMCS presets are here for legacy reasons. |
280ccbd to
3fa3d67
Compare
Hey @sebproell that makes sense, the preset is only useful for the workflow. I will prepare to create a workflow for that. |
Unfortunately, it is not possible to create a Darwin Docker image. Docker on MacOS runs a Linux virtual machine which then runs Linux-based images. The alternative is to use the GitHub-hosted runners with macOS. The free macos runner comes with 7 GB of memory (16 GB for Linux runner), which could pose a problem. Also, we would need to install all the dependencies during the workflow. There is also the possibility to create custom VM images. But this requires a paid runner and, from what I have read, is only possible for Linux- and Windows-based images. In summary, we could try to set up a workflow that uses a macos runner, install the dependencies during that workflow and build 4C. Due to the long runtime, I think we can only run this workflow once a week. But this should be fine. |
Hi @ppraegla, thanks for the useful information. Would you be so kind to add some commits to setup a priori the macos workflow, since you seem to be the one to know it best? Then I will add scripts to install the dependencies and test. In addition, the |
I can look into it some time next week.
This is described here. You basically compute the hash, update the hash in the workflows (e.g. see #1813), open a PR to build the new docker image, and then rerun the actions in the PR where you need the new dependencies. I suggest that we first set up the workflow for the macos runner. This will likely take some trial and error. Then we will see what we need to adapt in the dependency scripts. |
|
@vryy I added the stub for the macos workflow. In order to test the changes of this PR, you need to commit the workflow file to the main branch in your fork. Otherwise, the new workflow cannot be run. You will test the changes of this PR in your fork. Once the macos.yml workflow is on the main branch in your fork. You can go to Actions -> macOS build test -> Run workflow -> select your branch. Have a look what I did in my fork https://github.com/ppraegla/4C. The output of the workflow is here https://github.com/ppraegla/4C/actions/runs/23536820844/job/68514027187 I suggest that you try to get the workflow to pass in your fork. Once the macos workflow works, we can take care of the checks in this PR, i.e. updating the Docker image. |
|
The macos runner (M processor) only has 3 processors. So, the following test may fail |
0dd7e15 to
84b9c23
Compare
|
Hi @ppraegla thanks for providing the initial setup. I have added the compilation scripts and enabled the workflow. The current result is: The corresponding log is here: https://github.com/vryy/4C/actions/runs/23640661288/job/68860275643 |
|
Most of the tests failed because of unmatched tolerance, something we all knew about. Some differences are very closed, some are definitely something I will care about. Is there a way to specifically mark them as |
|
Nice work!
No, there is not yet such a functionality. However, if it are just slight adaptions to the test tolerances, we should just adapt them instead of skipping the tests. A further note, I would also try to activate all optional dependencies (like ArborX, VTK, GMSH) so that all tests are tested on MacOS |
|
I just had a quick look into the results of a browndyn test and the results are completely different. I guess this has something to do again with our random number generation. Could we have some way to add results for different platforms? I think dealii does something similar right? |
|
@vryy Most of the third-party library packages will be optional (also including parmetis, Umfpack, Superlu_dist, etc.). Would it make sense to try this with a minimal, no-dependency build first? |
I would rather opt to reduce the test to three procs. |
@maxfirmbach sorry I was OOF recently due to family matter. So far the tests look good except for the small tolerance ones, and browdyn. I think the minimal test should pass right away. Can you suggest a way to enable it? |
c84f6f3 to
4955f56
Compare
|
Finally, a workflow for macos is set up and passed, see https://github.com/vryy/4C/actions/runs/30552231074. There are still problematic tests but I disable that for macos by the flag |
e8fe466 to
d4511bc
Compare
ppraegla
left a comment
There was a problem hiding this comment.
I made some suggestions for further improvements, which can also be addressed in a follow-up PR. I'm fine with merging as-is, since the PR has been open for so long.
I mainly looked at the workflow and installation files. Others are more into the cmake stuff.
e9f95a7 to
8a51790
Compare
There was a problem hiding this comment.
Pull request overview
Adds Darwin/ARM build support, dependency installation, and weekly macOS CI coverage.
Changes:
- Adds Darwin presets and dependency scripts.
- Adds platform-aware test exclusions.
- Adds macOS build-and-test workflow.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/macos.yml |
Builds and tests 4C on macOS. |
CMakePresets.json |
Includes Darwin presets. |
presets/darwin/CMakePresets.json |
Defines Darwin release/debug builds. |
utilities/set_up_darwin.sh |
Prepares macOS system packages. |
dependencies/darwin/compile_dependencies.sh |
Orchestrates dependency builds. |
dependencies/darwin/mumps/install.sh |
Builds MUMPS. |
dependencies/darwin/parmetis/install.sh |
Builds ParMETIS. |
dependencies/darwin/qhull/install.sh |
Builds Qhull. |
dependencies/darwin/superlu_dist/install.sh |
Builds SuperLU_DIST. |
dependencies/darwin/trilinos/install.sh |
Builds Trilinos. |
cmake/functions/four_c_testing_functions.cmake |
Adds platform-based test skipping. |
tests/list_of_tests.cmake |
Excludes incompatible Darwin tests. |
src/reduced_lung/tests/4C_reduced_lung_aaa_approx_test.cpp |
Adds the required <numbers> include. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
c-p-schmidt
left a comment
There was a problem hiding this comment.
Looks reasonable to me; however, I need to admit that I'm not an expert on these workflow definitions.
Thank you for your efforts to enable 4C builds on MacOS.
|
One more thing. Have you looked into the linker warnings at the end of your build? They are not critical but might be worth a look. ` [1820/1829] Building CXX object tests/cut_test/CMakeFiles/cut_test.dir/cut_test_triangulateFacet.cpp.o |
|
@davidrudlstorfer You might want to have a look at the changes to the CMake functions related to testing. |
i'm not sure why this happens, since P/S: In the latest commit I added a line to create this folder in case it does not exist on github runner for any reason (Note that homebrew for Apple Silicon installed to |
8a51790 to
f1c1bc3
Compare
davidrudlstorfer
left a comment
There was a problem hiding this comment.
@c-p-schmidt thanks for the tag. I checked only the changes to the cmake testing functions.
One general question with the now excluded tests, it will probably make sense to create an issue with those and somehow tackle this in the future.
All the browniandyn tests fail because the random number generation is different on ARM processors, maybe we can adjust the random number generation or as already discussed add second results somehow like dealii does it.
What about the other tests that fail? the Cardiovascular0d tests are nothing super special on a first look, are there already suspicions why these fail? Same for the contact ones (especially because most of them succeed but then some fail)
I agree we shall open the issue to follow up with failed tests in the future. When it is done, one just remove the Some tests failed by just slightly off the tolerance. But again, I'd rather do it in separate PR. |
eeaa622 to
6481c1d
Compare
Description and Context
This PR adds cmake preset and necessary compilation scripts for the dependencies to compile 4C on the Mac/ARM system.
Related Issues and Pull Requests
#1771, #1779