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

Remove Makefiles due to deprecation #105

Merged
merged 4 commits into from
Jan 15, 2025
Merged

Conversation

diehlpk
Copy link
Contributor

@diehlpk diehlpk commented Dec 18, 2024

First pass to remove the option to build the examples using Make

Next step is to update the slides and copy the latest PDF to the repo which will be a separate pull request

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please confirm that you audited that for all removed Makefile there is indeed a CMakefile.txt and that it is being included via add_subdirectory (commented with a FIXME comment is fine)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is missing a cmake file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have CMakeLists.txt in all folders now. I just forgot to check the usecase folder.

 find  . -name "CMakeLists.txt"
./use-cases/core/median-finding/CMakeLists.txt
./Exercises/03/Begin/CMakeLists.txt
./Exercises/03/Solution/CMakeLists.txt
./Exercises/dualview/Begin/CMakeLists.txt
./Exercises/dualview/Solution/CMakeLists.txt
./Exercises/04/Begin/CMakeLists.txt
./Exercises/04/Solution/CMakeLists.txt
./Exercises/scatter_view/Begin/CMakeLists.txt
./Exercises/scatter_view/Solution/CMakeLists.txt
./Exercises/unordered_map/Begin/CMakeLists.txt
./Exercises/unordered_map/Solution/CMakeLists.txt
./Exercises/02/Begin/CMakeLists.txt
./Exercises/02/Solution/CMakeLists.txt
./Exercises/fortran-kokkosinterface/Begin/CMakeLists.txt
./Exercises/fortran-kokkosinterface/Solution/CMakeLists.txt
./Exercises/advanced_reductions/Begin/CMakeLists.txt
./Exercises/advanced_reductions/Solution/CMakeLists.txt
./Exercises/tools_minimd/CMakeLists.txt
./Exercises/kokkoskernels/GaussSeidel/Begin/CMakeLists.txt
./Exercises/kokkoskernels/GaussSeidel/Solution/CMakeLists.txt
./Exercises/kokkoskernels/TeamGemm/Begin/CMakeLists.txt
./Exercises/kokkoskernels/TeamGemm/Solution/CMakeLists.txt
./Exercises/kokkoskernels/CGSolve/Begin/CMakeLists.txt
./Exercises/kokkoskernels/CGSolve/Solution/CMakeLists.txt
./Exercises/kokkoskernels/InnerProduct/Begin/CMakeLists.txt
./Exercises/kokkoskernels/InnerProduct/Solution/CMakeLists.txt
./Exercises/kokkoskernels/SpGEMM/Begin/CMakeLists.txt
./Exercises/kokkoskernels/SpGEMM/Solution/CMakeLists.txt
./Exercises/kokkoskernels/BlockJacobi/Begin/CMakeLists.txt
./Exercises/kokkoskernels/BlockJacobi/Solution/CMakeLists.txt
./Exercises/kokkoskernels/CGSolve_SpILUKprecond/Begin/CMakeLists.txt
./Exercises/kokkoskernels/CGSolve_SpILUKprecond/Solution/CMakeLists.txt
./Exercises/kokkoskernels/SpILUK/Begin/CMakeLists.txt
./Exercises/kokkoskernels/SpILUK/Solution/CMakeLists.txt
./Exercises/kokkoskernels/GraphColoring/Begin/CMakeLists.txt
./Exercises/kokkoskernels/GraphColoring/Solution/CMakeLists.txt
./Exercises/vectorshift/Begin/CMakeLists.txt
./Exercises/vectorshift/Solution/CMakeLists.txt
./Exercises/simd/Begin/CMakeLists.txt
./Exercises/simd/Solution/CMakeLists.txt
./Exercises/simd_warp/Begin/CMakeLists.txt
./Exercises/simd_warp/Solution/CMakeLists.txt
./Exercises/virtualfunction/Begin/CMakeLists.txt
./Exercises/virtualfunction/Solution/CMakeLists.txt
./Exercises/mpi_exch/CMakeLists.txt
./Exercises/unique_token/Begin/CMakeLists.txt
./Exercises/unique_token/Solution/CMakeLists.txt
./Exercises/mpi_pack_unpack/Begin/CMakeLists.txt
./Exercises/mpi_pack_unpack/Solution/CMakeLists.txt
./Exercises/team_vector_loop/Begin/CMakeLists.txt
./Exercises/team_vector_loop/Solution/CMakeLists.txt
./Exercises/team_scratch_memory/Begin/CMakeLists.txt
./Exercises/team_scratch_memory/Solution/CMakeLists.txt
./Exercises/01/Begin/CMakeLists.txt
./Exercises/01/Solution/CMakeLists.txt
./Exercises/multi_gpu_cuda/Begin/CMakeLists.txt
./Exercises/multi_gpu_cuda/Solution/CMakeLists.txt
./Exercises/team_policy/Begin/CMakeLists.txt
./Exercises/team_policy/Solution/CMakeLists.txt
./Exercises/parallel_scan/Begin/CMakeLists.txt
./Exercises/parallel_scan/Solution/CMakeLists.txt
./Exercises/mdrange/Begin/CMakeLists.txt
./Exercises/mdrange/Solution/CMakeLists.txt
./Exercises/subview/Begin/CMakeLists.txt
./Exercises/subview/Solution/CMakeLists.txt
./Exercises/random_number/Begin/CMakeLists.txt
./Exercises/random_number/Solution/CMakeLists.txt
./Exercises/mpi_heat_conduction/Solution/CMakeLists.txt
./Exercises/tasking/Begin/CMakeLists.txt
./Exercises/tasking/Solution/CMakeLists.txt

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you confirm that these not just exist but also are added? #105 (review)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I ran find . -name "CMakeLists.txt" -exec git add "{}" ";", all of them should be added.

Copy link
Contributor

@cwpearson cwpearson Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# TODO: hpcbind does not use cmake
# TODO: instances does not use cmake
# TODO: parallel_scan seems broken
# TODO: simd_warp seems broken
# TODO: subview seems broken
# TODO: vectorshift needs Kokkos Remote Spaces
# TODO: kokkoskernels/CGSolve_SpILUKprecond needs to know where Kokkos Kernels source directory is
# TODO: kokkoskernels/SpILUK needs to know where Kokkos Kernels source directory is
# TODO: kokkoskernels/TeamGemm seems broken
# TODO: mpi_heat_conduction/no-mpi does not use cmake

hpcbind, instances, and mpi_heat_conduction/no-mpi do not have a CMakeLists.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwpearson I added these CMakefiles with comments to fix these.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with merging this if you open an issue against me to get these new CMake files building in the CI.

@dalg24 dalg24 changed the title Remove Makefiles du to deprication Remove Makefiles du to deprecation Dec 19, 2024
@dalg24 dalg24 changed the title Remove Makefiles du to deprecation Remove Makefiles due to deprecation Dec 19, 2024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with merging this if you open an issue against me to get these new CMake files building in the CI.

@cwpearson
Copy link
Contributor

@dalg24 we're working on reviving our Windows dev machine at Sandia, at which point I can fix the Windows CI. In the mean time, I don't think we should block on it.

@dalg24 dalg24 merged commit c2cf41b into kokkos:main Jan 15, 2025
3 of 4 checks passed
@diehlpk diehlpk deleted the remove_make_files branch January 15, 2025 23:53
@dalg24 dalg24 mentioned this pull request Jan 16, 2025
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.

3 participants