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

Add missing dependencies on installed DDC #665

Merged
merged 4 commits into from
Oct 24, 2024
Merged

Conversation

tpadioleau
Copy link
Member

@tpadioleau tpadioleau commented Oct 16, 2024

  • declare missing dependencies in DDCConfig.cmake
  • install missing FindLAPACKE.cmake, required by DDC when using splines
  • add CI test

It seems to be working but could be further improved in future PR.

  • The headers related to fft, splines and the PDI wrapper of DDC are being installed whatever the values of the cmake options. I think it could improved with some reorganization.

  • Cmake targets about the fft and splines are missing.

  • We should look at cmake components. As long as the user can adapt it looks ok

find_package(DDC REQUIRED)

add_executable(main main.cpp)
target_link_libraries(main PRIVATE DDC::DDC)

if(DDC_BUILD_PDI_WRAPPER)
    target_link_libraries(main PRIVATE DDC::PDI_Wrapper)
endif()

but as soon as a user requires an option this looks bad

find_package(DDC REQUIRED)

if(NOT DDC_BUILD_KERNELS_FFT)
    message(FATAL_ERROR "Expecting DDC_BUILD_KERNELS_FFT=ON")
endif()
  • DDC lacks a header configuration file to list enabled options. These are passed as "compile definition" but pollute the user side.

@tpadioleau tpadioleau self-assigned this Oct 16, 2024
@tpadioleau tpadioleau linked an issue Oct 16, 2024 that may be closed by this pull request
@tpadioleau tpadioleau marked this pull request as draft October 16, 2024 12:33
@tpadioleau tpadioleau force-pushed the 307-make-ddc-installable branch 3 times, most recently from 0fe2375 to d64e70b Compare October 16, 2024 16:16
@tpadioleau tpadioleau force-pushed the 307-make-ddc-installable branch 5 times, most recently from ba0d26e to 565525f Compare October 16, 2024 20:54
@tpadioleau tpadioleau marked this pull request as ready for review October 17, 2024 08:08
@tpadioleau tpadioleau force-pushed the 307-make-ddc-installable branch 2 times, most recently from 87254c5 to 561f71c Compare October 23, 2024 14:45
.github/workflows/tests.yml Show resolved Hide resolved
cmake/DDCConfig.cmake.in Outdated Show resolved Hide resolved
cmake/DDCConfig.cmake.in Show resolved Hide resolved
.github/workflows/tests.yml Show resolved Hide resolved
@yasahi-hpc
Copy link
Member

If you added install_test, should you also apply format check to that directory?

source: 'benchmarks/ examples/ include/ddc/ tests/'

@tpadioleau
Copy link
Member Author

If you added install_test, should you also apply format check to that directory?

source: 'benchmarks/ examples/ include/ddc/ tests/'

Indeed good catch

@tpadioleau tpadioleau merged commit 806d786 into main Oct 24, 2024
56 checks passed
@tpadioleau tpadioleau deleted the 307-make-ddc-installable branch October 24, 2024 19:28
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.

Make DDC installable
3 participants