Compiling on conda-forge's Macs#59
Merged
sanathkeshav merged 8 commits intoDataAnalyticsEngineering:developfrom May 20, 2025
Merged
Compiling on conda-forge's Macs#59sanathkeshav merged 8 commits intoDataAnalyticsEngineering:developfrom
sanathkeshav merged 8 commits intoDataAnalyticsEngineering:developfrom
Conversation
+ gcc and clang-linux working, same speed for test
main.cpp:32:9: warning: delete called on 'Matmodel<1>' that is abstract but has non-virtual destructor [-Wdelete-abstract-non-virtual-dtor] 32 | delete matmodel; given through clang. + also applied the same to Solver
+ uncomment compile options
+ remove unnecessary comment
+ remove doubled CMake min version
+ introduce CXX_FLAGS for gcc in preset
+ also in parts for Clang, -fmax-errors does not exist there
+ reintroduce RPATH-logic for Apple -> don't understand it
+ add condition to mac presets -> only loose, as darwin is open-source and could NOT be apple... but seems to be standard vs APPLE-check....
IshaanDesai
approved these changes
May 19, 2025
Collaborator
There was a problem hiding this comment.
Thanks for the valuable contribution @claudiushaag ! I just have one minor comment regarding consistency. I would suggest in keeping the documentation in the repository, perhaps as notes in a sub folder. It is valuable content.
Shall we also add CI to check conda builds? Perhaps it makes sense.
Collaborator
|
Please don't forget to add a change log entry. |
sanathkeshav
approved these changes
May 20, 2025
6960e64
into
DataAnalyticsEngineering:develop
7 checks passed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of our effort to introduce
osxversions ofFANSviaconda-forge.The recipe-update was done in this PR
Restrictive to this process was the fact that conda-forge only builds against Clang on MacOS, and crosscompiles for arm64.
I did this with little to no deep knowledge of CMake and compilation in general - sorry if I messed up any working aspect and introduced some regression. Would be open to fix this obviously.
The overall strategy was:
x86arm64.osx-64andosx-arm64version ofFANS.Steps 2 and 3 were mainly carried out in the conda recipe itself. The result of the first step is this PR.
Step 4 and 5 are still ahead of us.
Introduced Changes
3.21for PresetsMatmodelandSolver, as those crashed onosxasIllegal Instructionand also showed up as a compiler warning:AVX2andFMAdetection logic - those CPUs are just to old.Todos
I don't know how these changes will interact with all other setup in the repo itself:
so please check. Would love PRs on the branch, so I can test the conda-CI.
There is also a Markdown-File which I generated to give a short introduction into how to use the Presets. I would delete that before merging, but maybe it serves a purpose right now as inspo for docs, README and testing...