Skip to content

Fix the build; make layout_transpose and conjugated_accessor conform - #269

Merged
mhoemmen merged 3 commits into
kokkos:mainfrom
mhoemmen:fix-build-202404
Apr 4, 2024
Merged

Fix the build; make layout_transpose and conjugated_accessor conform#269
mhoemmen merged 3 commits into
kokkos:mainfrom
mhoemmen:fix-build-202404

Conversation

@mhoemmen

@mhoemmen mhoemmen commented Apr 3, 2024

Copy link
Copy Markdown
Contributor

This fixes the build, which is currently broken, thus blocking any new features.

  1. Change this repository so that it no longer uses or depends on std::experimental.

  2. Fix layout_transpose to conform with the Standard. It had implemented an older version of mdspan that used size_type instead of index_type.

  3. Rename accessor_conjugate to conjugated_accessor and otherwise bring it in line with the Standard. Improve tests for conjugated.

This is rebased atop #264 which I submitted in January but hasn't yet been merged. Please either merge that one first, or close that one and merge this one.

@mhoemmen
mhoemmen requested review from crtrott, dalg24 and nliber April 3, 2024 15:44
Comment thread tests/native/transposed.cpp

@dalg24 dalg24 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I scrolled through the changes of the last 3 commits (that is excluding the changes from the PR that I just merged) and it looks good besides https://github.com/kokkos/stdBLAS/pull/269/files/2584d02d3230805525db529d39033982c3120fc2..ca3f2614dfa5d6edb2abb7bdcbc508d90a56eeef#r1550159723
If you withdraw these addition I will approve and merge

mhoemmen added 3 commits April 3, 2024 12:37
layout_transpose was imitating an old version of mdspan.
For example, it used size_type instead of index_type
as the return type of operator().  That made tests for
the return type of transposed fail (because the transposed
layout mapping's extents_type had the wrong index_type).
This commit fixes that.

Fix transposed(layout_stride) build warning.

Add tests for transposed of layout_right and
transposed of layout_stride.
Namespaces: The tests (in tests/native) and examples
no longer assume that linalg and mdspan live in the
std::experimental namespace.

Includes: This implies including mdspan/mdspan.hpp
instead of experimental/mdspan.hpp.
and otherwise bring it in line with the Standard.

Add conjugated test for a nonarithmetic real type.
"Real type" in this case just means that conj of it
is not ADL-findable.

Add conjugated test for a custom complex number.
"Custom complex number" in this case just means
that conj of it is ADL-findable.
@mhoemmen

mhoemmen commented Apr 3, 2024

Copy link
Copy Markdown
Contributor Author

@dalg24 wrote:

I scrolled through the changes of the last 3 commits (that is excluding the changes from the PR that I just merged) and it looks good besides https://github.com/kokkos/stdBLAS/pull/269/files/2584d02d3230805525db529d39033982c3120fc2..ca3f2614dfa5d6edb2abb7bdcbc508d90a56eeef#r1550159723
If you withdraw these addition I will approve and merge

Fixing the merge conflicts seems to have made this link invalid. My understanding is that you were referring to the new tests in tests/native/transposed.cpp. Those tests only exercise current P1673R13 features. As I explained above,

  • transposed of layout_left is layout_right,
  • transposed of layout_right is layout_left, and
  • transposed of layout_stride is layout_stride.

That has always been true; this has nothing to do with any new proposals.

@mhoemmen
mhoemmen merged commit 06e90a5 into kokkos:main Apr 4, 2024
@mhoemmen
mhoemmen deleted the fix-build-202404 branch April 4, 2024 04:04
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