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

Use Kokkos 4.4.01 with mdspan coming from Kokkos #651

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

tpadioleau
Copy link
Member

@tpadioleau tpadioleau commented Sep 17, 2024

  1. update submodule Kokkos to version 4.4.01,
  2. remove submodule mdspan,
  3. change std::experimental::mdspan to Kokkos::mdspan,
  4. convert potential layout_[left|right]_padded returned by submdspan to layout_stride,
  5. Kokkos Kernels also needs to be updated, see with @yasahi-hpc

The reason I use the strategy 3. is that submdspan may return a layout_left_padded/layout_right_padded layout. Although these layouts should map to Kokkos LayoutLeft/LayoutRight, I could not find in Kokkos 4.4.01 any public API to construct a Kokkos layout with the padding value.

@tpadioleau tpadioleau linked an issue Sep 17, 2024 that may be closed by this pull request
@tpadioleau tpadioleau force-pushed the 604-conflict-between-mdspan-and-kokkos-4400 branch 7 times, most recently from bdfa078 to 0672aa0 Compare September 20, 2024 11:26
@tpadioleau tpadioleau marked this pull request as ready for review September 24, 2024 19:17
@tpadioleau tpadioleau force-pushed the 604-conflict-between-mdspan-and-kokkos-4400 branch 4 times, most recently from 6c80bc1 to d3ad777 Compare September 30, 2024 09:26
@pzehner
Copy link
Member

pzehner commented Sep 30, 2024

I could not find in Kokkos 4.4.01 any public API to construct a Kokkos layout with the padding value.

@tpadioleau Did you open an issue on Kokkos about this?

@tpadioleau
Copy link
Member Author

@tpadioleau Did you open an issue on Kokkos about this?

Not yet, we should do it at some point. It is not critical now that we have found a workaround.

Copy link
Member

@yasahi-hpc yasahi-hpc left a comment

Choose a reason for hiding this comment

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

Looks good to me except for minor changes. I also have some questions

include/ddc/chunk.hpp Show resolved Hide resolved
include/ddc/chunk_span.hpp Outdated Show resolved Hide resolved
include/ddc/chunk_span.hpp Outdated Show resolved Hide resolved
include/ddc/chunk_span.hpp Outdated Show resolved Hide resolved
include/ddc/kernels/splines/view.hpp Outdated Show resolved Hide resolved
Copy link
Member Author

@tpadioleau tpadioleau left a comment

Choose a reason for hiding this comment

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

thanks for the review

include/ddc/chunk.hpp Show resolved Hide resolved
include/ddc/chunk_span.hpp Outdated Show resolved Hide resolved
include/ddc/chunk_span.hpp Outdated Show resolved Hide resolved
@tpadioleau tpadioleau force-pushed the 604-conflict-between-mdspan-and-kokkos-4400 branch from c16ad68 to 49c8924 Compare September 30, 2024 16:52
@tpadioleau tpadioleau force-pushed the 604-conflict-between-mdspan-and-kokkos-4400 branch from 49c8924 to 3d803f3 Compare October 1, 2024 06:06
@tpadioleau
Copy link
Member Author

tpadioleau commented Oct 1, 2024

@acalloo do you still want to do a review or should I merge ?

Edit: Ansar is ok to merge

@tpadioleau tpadioleau merged commit 2e8e3ed into main Oct 2, 2024
55 of 56 checks passed
@tpadioleau tpadioleau deleted the 604-conflict-between-mdspan-and-kokkos-4400 branch October 2, 2024 08:02
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.

Conflict between mdspan and Kokkos 4.4
3 participants