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

Configure: support KOKKOS when found with PETSc #14677

Merged
merged 1 commit into from Jan 17, 2023

Conversation

stefanozampini
Copy link
Contributor

@stefanozampini stefanozampini commented Jan 13, 2023

This still require grabbing the compile flags and some special CUDA handling?

I'm not a CMAKE expert. I profoundly hate it. So if any CMAKE expert wants to pitch in and provide comments/suggestions/improvements, please go ahead @luca-heltai @bangerth

This still require grabbing the compile flags and some special CUDA handling?
@luca-heltai
Copy link
Member

I think @tamiko is our expert here. :)

@masterleinad
Copy link
Member

Does PETSc bundle Kokkos or does it link to an external installation?

@stefanozampini
Copy link
Contributor Author

We don't ship Kokkos source code. We link against an existing installation (that can also be automatically downloaded and built during PETSc configure)

Copy link
Member

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

We don't ship Kokkos source code. We link against an existing installation (that can also be automatically downloaded and built during PETSc configure)

In that case, I would rather require that we don't allow using the bundled Kokkos package but require Kokkos to be found as an external package and not import Kokkos flags from PETSc but rather from Kokkos directly. We treat Trilinos differently because they can't yet use an external Kokkos installation.

This way, we could also allow building deal.II with PETSc with Kokkos and Trilinos with Kokkos assuming that PETSc could use Trilinos' bundled Kokkos installation.

@@ -26,7 +26,7 @@ set(KOKKOS_DIR "" CACHE PATH "An optional hint to a Kokkos installation")
set_if_empty(KOKKOS_DIR "$ENV{KOKKOS_DIR}")


if(DEAL_II_TRILINOS_WITH_KOKKOS)
if(DEAL_II_TRILINOS_WITH_KOKKOS OR DEAL_II_PETSC_WITH_KOKKOS)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(DEAL_II_TRILINOS_WITH_KOKKOS OR DEAL_II_PETSC_WITH_KOKKOS)
if(DEAL_II_TRILINOS_WITH_KOKKOS)

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'm ok with this change. We probably need more automation and to provide hints to find_package to look into PETSc makefile variables KOKKOS_INCLUDE and KOKKOS_LIB

if(NOT PETSC_PETSCVARIABLES MATCHES "-NOTFOUND")

@tamiko
Copy link
Member

tamiko commented Jan 16, 2023

A couple of thoughts:

  • I think that this PR is a good starting point for a sanity check: force external kokkos if PETSc is configured with Kokkos. We already do something similar for Trilinos (Similarly to our MPI sanity checks we could make sure that we configure against one external kokkos).
  • For actually finding a Kokkos installation "bundled" with PETSc we should make sure that the FindDEAL_II_KOKKOS module has the right search paths (via appropriate hints). I am happy to look into this in the very near future:

That said: I really want to get the CMake refactoring done now and I suggest the we hold off with changes to external features for about 2-4 weeks until said refactoring is done. Otherwise we will have nasty inflight collisions. Or have to fix problems twice.

@tamiko
Copy link
Member

tamiko commented Jan 16, 2023

Depends on #12374

@stefanozampini
Copy link
Contributor Author

stefanozampini commented Jan 16, 2023

@tamiko This small change allows me to build and run using my PETSc install with KOKKOS dependency, otherwise there are a bunch of errors at runtime since PETSc and deal.II uses a different KOKKOS version. Can we merge this as is for now?

Thanks for offering to improve it in the future, very much appreciated!

@tamiko
Copy link
Member

tamiko commented Jan 16, 2023

@stefanozampini Fair enough.

@luca-heltai
Copy link
Member

/rebuild

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants