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

Update and simplify cmake #495

Conversation

DiamonDinoia
Copy link
Collaborator

@DiamonDinoia DiamonDinoia commented Jul 18, 2024

Dear all,

I made some changes on cmake to remove the _static to finufft and cleanup other things in the process.

Changelog:

  • -march=native like on msvc
  • static libraries by default only (shared replaces static)
  • flags are detected by cmake and all the supported ones are enabled

Known issues:

  • move msvc march=native to a separate file
  • build GPU library does not depend on CPU anymore
  • GPU should not depend on FFTW
  • clang-cl declspec issue

@DiamonDinoia
Copy link
Collaborator Author

@lu1and10 do you know why windows clang-cl is not building?

@lu1and10
Copy link
Member

@lu1and10 do you know why windows clang-cl is not building?

I'm not sure, it seems too much going on with the cmake changes, need to take a look in detail.

@DiamonDinoia
Copy link
Collaborator Author

I think I fixed everything. I think we should use the matrix I use for testing python to test cmake-ci too. This way we can test all os and all compilers.

@lu1and10 lu1and10 changed the base branch from master to gencoef July 18, 2024 19:23
@DiamonDinoia DiamonDinoia changed the base branch from gencoef to master July 19, 2024 20:34
@janden
Copy link
Collaborator

janden commented Jul 22, 2024

To reduce issues with CMakeLists style, etc., would it make sense to use a tool like cmake-format (https://github.com/cheshirekow/cmake_format) in our pre-commit hook?

perftest/cuda/bench.py Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
src/cuda/CMakeLists.txt Show resolved Hide resolved
@DiamonDinoia DiamonDinoia merged commit 4a176c4 into flatironinstitute:master Jul 31, 2024
87 of 88 checks passed
@DiamonDinoia DiamonDinoia deleted the 388-cmake-build-only-static-by-default branch July 31, 2024 22:04
@lu1and10
Copy link
Member

One note for future: @DiamonDinoia finds that on windows with msvc cl, ducc0 has to compile with /fp:fast, otherwise some tests(run_finufft3d_test_float, run_finufft3dmany_test_float) may fail because of the resulting error is larger than the tolerance.

On the other hand, finufft on windows with msvc cl should not compile with flag /fp:fast, with /fp:fast the test run_dumbinputs_double will result in segfault, because /fp:fast makes values (NaN, +infinity, -infinity, -0.0) may not be propagated or behave strictly according to the IEEE-754 standard

We may need to add this note in the installation doc.

@ahbarnett
Copy link
Collaborator

ahbarnett commented Aug 1, 2024 via email

@DiamonDinoia
Copy link
Collaborator Author

I added a warning in the docs.

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.

4 participants