Skip to content

Kokkos complex_align variant, Trilinos+PETSc enforcement for Kokkos~complex_align#47686

Merged
psakievich merged 4 commits intospack:developfrom
finkandreas:kokkos-complex_align
Nov 23, 2024
Merged

Kokkos complex_align variant, Trilinos+PETSc enforcement for Kokkos~complex_align#47686
psakievich merged 4 commits intospack:developfrom
finkandreas:kokkos-complex_align

Conversation

@finkandreas
Copy link
Copy Markdown
Contributor

This PR tackles two problems, but I felt that I will add it to a single PR. Let me know if I must split it into two.
The first issue is the missing variant of complex_align on Kokkos. Trilinos, when built with the builtin kokkos version will set complex_align to OFF, which is the opposite of what the default in the Kokkos package is.
This PR adds the variant with default value true, but enforcing it to false, when building trilinos.
Also PETSc, when building for complex scalars and kokkos support enforces that the variant complex_align is set to false.

The second issue this PR tackles is the incorrect dependency on the external Kokkos package for trilinos. Basically this line

        if spec.satisfies("@14.4.0 +kokkos"):

This is missing the upper boundary, and only restricts on the very specific trilinos version.
This leads to trilinos pulling in the internal kokkos version, and one would end up with two kokkos libraries. One provided by kokkos (correct) and one provided by trilinos (incorrect).
Also I have the feeling that when we enable trilinos+kokkos, we actually also want to unbundle kokkos-kernels from trilinos, hence I added this too.
All in all, using this PR we can build a version of trilinos that links to kokkos and kokkos-kernels without pulling in the bundled versions inside trilinos.

Without this PR, building an environment with a view would lead to multiple libkokkos_*.so, provided by trilinos, kokkos and kokkos-kernels.

haampie
haampie previously approved these changes Nov 20, 2024
@balay
Copy link
Copy Markdown
Contributor

balay commented Nov 20, 2024

cc: @jczhang07

Copy link
Copy Markdown
Member

@haampie haampie left a comment

Choose a reason for hiding this comment

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

Thanks, I'll let the trilinos/kokkos folks have a look, but in terms of style it's a big improvement as well.

@jczhang07
Copy link
Copy Markdown

cc: @jczhang07

@balay yes, we added -DKokkos_ENABLE_COMPLEX_ALIGN=OFF in https://gitlab.com/petsc/petsc/-/merge_requests/5038. I regret I did not log the error message in the MR. I guess it was because when building kokkos on CPUs, we want interoperability between std::complex and kokkos::complex.

@psakievich
Copy link
Copy Markdown
Contributor

This seems fine to me, but I'm not a core member of the kokkos team.

Copy link
Copy Markdown
Contributor

@cedricchevalier19 cedricchevalier19 left a comment

Choose a reason for hiding this comment

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

Ok for the kokkos recipe.

@balay
Copy link
Copy Markdown
Contributor

balay commented Nov 29, 2024

This change is triggering [email protected] build error with ./bin/spack install [email protected]%[email protected]

Before this change:
spack-build-out-good-truncated.txt

After this change:
spack-build-out-bad.txt

fryeguy52 pushed a commit to fryeguy52/spack that referenced this pull request Dec 17, 2024
kshea21 pushed a commit to kshea21/spack that referenced this pull request Dec 26, 2024
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Feb 5, 2025
nkoukpaizan pushed a commit to nkoukpaizan/spack that referenced this pull request Apr 4, 2025
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.

6 participants