-
Notifications
You must be signed in to change notification settings - Fork 41
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
AppleClang15 patch #116
AppleClang15 patch #116
Conversation
… longer used on arm architectures\n- NEON SIMD option only auto selected on arm architectures, defaults to AVX otherwise\n- polar() function now defaults arg2 to 0.0 rather than throwing an error/ignoring overload\n- <StdCompatibility.cpp> auto included for arm & appleclang15\n - One test changed in Complex_test.cpp to prevent overload error. This is a temporary change and the test will be restored once the error related to std::norm is fixed\n - Added Mac Clang15 workflow (debug & release) to ci.yml (yet to test)
- Proper overload for std::norm now selected when norm is called with type param. - Added NEON SIMD option to drop down menu for CMake GUI - Restored test I previously changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. Please address the review comments before we can merge this.
Regarding the failing Windows toolset 14.1 build, could you change the CI/CD so that MSVC 14.0 and 14.1 are using the runner image |
- M1 workflow no longer needlessly installs llvm15 - Fixed erroneous formatting error & redundant flag addition - SIMD: NEON -> APPLE_M1, & added NATIVE - norm overloads now use = Constructor() rather than = 0 where relevant - Removed redundant include of <StdCompatibility.hpp> in XAD.hpp - Windows 14.1 build now uses 2019 runner
Given the differences between the different compiler versions and Mac architectures, I believe we should extend the CI/CD build into a wider matrix build (similar to Linux/Windows). You can see here that both Arm and Intel based runners are available, and we have Clang 14/15 to test with on each. Plus Debug/Release and coverage. If you want to do that as part of this PR, that would be great. Otherwise it could also be done in a separate one. |
- Changed APPLE_M1 flag -march=apple-m1 -> -march=armv8.5-a - Removed redundant GNU branch in SetupCompiler.cmake - Added better system resolve (Darwin/Linux) in SetupOptions.cmake - Changed overload for norm as to enable if type is XAD rather than disable if type is a c++ native type This fix intends to fix the flag issue for the macoslatest workflow. It may not; in which case this task is relayed it to the next commit. Next fix will fix baseline coverage issues for Mac (Debug)
- Merged macos-13 & macos-latest into wider matrix build. - Changed Mac (Debug) to use [email protected] rather than [email protected] to try fix coverage baseline issue. - Changed Expressions_test 1686 to use tolerance value rather than DOUBLE_EQ. The lcov version downgrade may not fix coverage baseline, in which case that change will be reverted
Pull Request Test Coverage Report for Build 9364361988Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is nearly ready now - if you could only look after the two small comments that are left, we'll get this merged into the main branch.
1b8be41
into
auto-differentiation:main
Description
Mac Silicon compatibility fix. Aims to resolve Apple Silicon Support #99. Includes:
NATIVE
andAPPLE_M1
SIMD optionsmacos-13
&macos-latest
, Debug and ReleaseAdditional non-issue related changes:
2.1
->1.14
to repair coverage baseline issusesType of change