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

support use of mdspan/mdarray.hpp header #376

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from

Conversation

cwsmith
Copy link

@cwsmith cwsmith commented Jan 15, 2025

Using #include "mdspan/mdarray.hpp" provided by mdspan fails with the following error:

In file included from /space/cwsmith/mdspan/buildMdsCuda/install/include/mdspan/mdarray.hpp:29,
                 from /space/cwsmith/mdspan/mdarrayReproducer/mdarrayReproducer.cpp:17:         
/space/cwsmith/mdspan/buildMdsCuda/install/include/mdspan/../experimental/__p1684_bits/mdarray.hpp:19:10: fatal error: ../mdspan: No such file or directory
   19 | #include "../mdspan"                                                                                                            
      |          ^~~~~~~~~~~                                                                                                            
compilation terminated. 

A reproducer (mdarrayReproducer.cpp) is below:

#include "mdspan/mdarray.hpp"
int main(int argc, char* argv[]) {
  return 0;
}

and its CMakeLists.txt:

cmake_minimum_required(VERSION 3.16)
project(mdarrayReproducer CXX)
find_package(mdspan REQUIRED)
add_executable(reproducer mdarrayReproducer.cpp)
target_link_libraries(reproducer std::mdspan)

This PR fixes the compilation error, but I'm not sure if it is the correct fix. If it is merged and then brought into downstream Kokkos, it will resolve the issue discussed in kokkos/kokkos#7671.

All mdspan tests passed when using the following cmake commands with GCC 12.3.0 and CUDA 12.1.1.

bdir=$PWD/buildMdsCuda
cmake -S mdspan -B $bdir \
  -DCMAKE_INSTALL_PREFIX=$bdir/install \
  -DMDSPAN_CXX_STANDARD=17 \
  -DMDSPAN_ENABLE_TESTS=on \
  -DMDSPAN_ENABLE_EXAMPLES=on \
  -DMDSPAN_ENABLE_CUDA=on \
  -DCMAKE_CUDA_FLAGS=--expt-relaxed-constexpr \
  -DCMAKE_CUDA_ARCHITECTURES=86 
cmake --build buildMdsCuda -j20 --target install
ctest --test-dir buildMdsCuda/

@@ -16,7 +16,7 @@

#pragma once

#include "../mdspan"
#include "mdspan/mdspan.hpp"
Copy link
Contributor

@nmm0 nmm0 Jan 15, 2025

Choose a reason for hiding this comment

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

I think this would work with just "../mdspan/mdspan.hpp".

The issue here is we switched the main headers to mdspan.hpp instead of mdspan, and I guess we missed this. I guess this only failed in Kokkos because we don't install the legacy mdspan header

Copy link
Author

Choose a reason for hiding this comment

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

#include "../../mdspan/mdspan.hpp" worked. Should I push that change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think that would be good thank you!

Copy link
Author

Choose a reason for hiding this comment

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

9582f5b has the change

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.

2 participants