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

Sacado header brings in Kokkos and breaks compilation of deal.II with CUDA #6423

Closed
davydden opened this issue May 2, 2018 · 7 comments
Closed

Comments

@davydden
Copy link
Contributor

davydden commented May 2, 2018

as discussed in spack/spack#7957

include/deal.II/base/numbers.h includes Sacado.hpp when compiled with Trilinos. That, in turn, leads to a chain that ends with including Kokkos_Macros.hpp and gives compilation error when deal.II is built with CUDA:

 >> 1791    /home/xap/local/opt/spack/opt/spack/linux-ubuntu16.04-x86_64/gcc-5.4.0/trilinos-12.12.1-yosvheg3dyhiq3gbrbfnmzwe7rtgvdjo/include/impl/Kokkos_Err
             or.hpp(76): error: namespace "Kokkos::Impl" has no member "cuda_abort"

Links to similar Kokkos errors are here.

As a summary, it's a combination of dealii+CUDA and trilinos+sacado+kokkos.
In the original issue it was suggested to try using Sacado_No_Kokkos.hpp.

EDIT: Sacado_No_Kokkos.hpp still includes kokkos headers and gives the same compiling error.

@davydden davydden added the GPU label May 2, 2018
@davydden davydden added this to the Release 9.0 milestone May 2, 2018
@jppelteret
Copy link
Member

@davydden To reply to your comment spack/spack#7957 (comment)

since you work with AD, do you know anything about Sacado_No_Kokkos.hpp?

No, I don't know anything about Sacado_No_Kokkos.hpp (although I do vaguely recall seeing this while browsing the source code). I think that the best option here that works for all of us would be to replace each instance of

#include <Sacado.hpp>

with

#ifdef DEAL_II_WITH_CUDA
#include <Sacado_No_Kokkos.hpp>
#else
#include <Sacado.hpp>
#endif

There are only a few places in the library where sacado.hpp is included:

  • base/numbers.h
  • differentiation/ad/sacado_number_types.h
  • differentiation/ad/sacado_product_types.h
  • step-33
  • some tests

I can create a PR which swaps the headers, but I don't have any means to tests as of now.

Thanks, I would appreciate it if you were to open a PR for this!

@Rombur
Copy link
Member

Rombur commented May 3, 2018

As a summary, it's a combination of dealii+CUDA and trilinos+sacado+kokkos.

Note that dealii+cuda+sacado works fine. I have a code that is using this configuration without any problem. The bug only appears when trilinos is compiled with kokkos which is the case if you use tpetra.

@Rombur
Copy link
Member

Rombur commented May 3, 2018

The bug is really in Kokkos and there is nothing we can do about it. I have open an issue.

@ibaned
Copy link

ibaned commented May 3, 2018

kokkos/kokkos#1607
I'll just mention that this kind of usage is somewhere between very rare and unprecedented for Kokkos, so there may be many obstacles but we'll see what we can do. I assume you're not really expecting any Kokkos functions or data structures to actually work inside CUDA kernels? Otherwise it really would be best to just enable the Kokkos CUDA backend.

@aprokop
Copy link

aprokop commented May 3, 2018

Everything seems to compile when I disable Kokkos and the new stack in Trilinos, including Sacado. But this is only for Trilinos master. For 12.12 branch, there is some bug in Sacado that unconditionally uses Kokkos even if Trilinos is not compiled with it. I'm not sure it's worth pursuing. I'll comment some more tomorrow in the spack issue.

@Rombur
Copy link
Member

Rombur commented May 3, 2018

I assume you're not really expecting any Kokkos functions or data structures to actually work inside CUDA kernels?

No, we actually only use the Epetra stack. The problem is if someone decides to install Trilinos with both the Epetra and the Tpetra stack. For some reason, Kokkos gets pull in and the problem arises.

@masterleinad
Copy link
Member

This is now fixed upstream and there is nothing to be done here anymore.

@Rombur Rombur moved this from In Progress to Done in CUDA support Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

6 participants