-
Notifications
You must be signed in to change notification settings - Fork 99
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
Nightly Trilinos failures, all backends: error: no template named 'SerialTrsvInternalUpper' in namespace 'KokkosBatched' #2456
Comments
Yes, I think we moved some implementation details into the Impl namespace. Glad to see that Trilinos is using said details directly instead of the public API... |
@ndellingwood @lucbv Current KokkosBatched::SerialTrsvInternalUpper<KokkosBatched::Algo::Trsv::Unblocked>::invoke(false,
A0.extent(0),
1.0,
A0.data(), A0.stride_0(), A0.stride_1(),
b.data(), b.stride_0()); Updated KokkosBatched::SerialTrsv<KokkosBatched::Uplo::Upper, KokkosBatched::Trans::NoTranspose,
KokkosBatched::Diag::NonUnit, KokkosBatched::Algo::Trsv::Unblocked>::invoke(1.0, A0, b); |
@yasahi-hpc what you suggest above makes sense however I would also add the following at the end of
That way users that may be relying on this will get some time to react to our move and should be able to fix things before we actually remove the non-Impl version completely. |
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
@yasahi-hpc thanks for the suggestion, I tried testing the intrepid2 change with Trilinos (created Draft PR here trilinos/Trilinos#13671) , but I am still seeing issues:
The line 549 refers to the Trilinos PR trilinos/Trilinos#13671 , can you help advise? |
@ndellingwood Sorry for the inconvenience. Current #ifdef HAVE_INTREPID2_KOKKOSKERNELS
#include "KokkosBatched_QR_Serial_Internal.hpp"
#include "KokkosBatched_ApplyQ_Serial_Internal.hpp"
#include "KokkosBatched_Trsv_Serial_Internal.hpp"
#include "KokkosBatched_Util.hpp"
#endif Updated #ifdef HAVE_INTREPID2_KOKKOSKERNELS
#include "KokkosBatched_QR_Serial_Internal.hpp"
#include "KokkosBatched_ApplyQ_Serial_Internal.hpp"
#if KOKKOS_VERSION >= 40599
#include "KokkosBatched_Trsv_Decl.hpp"
#else
#include "KokkosBatched_Trsv_Serial_Internal.hpp"
#endif
#include "KokkosBatched_Util.hpp"
#endif |
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Thanks @yasahi-hpc for the fast response! I updated the PR and will let you know how the next run goes |
@yasahi-hpc unfortunately I am still seeing compilation issues with intrepid2 with these changes https://github.com/trilinos/Trilinos/pull/13671/files
|
@ndellingwood Current KokkosBatched::SerialTrsvInternalUpper<KokkosBatched::Algo::Trsv::Unblocked>::invoke(false,
A.extent(0),
1.0,
A.data(), A.stride_0(), A.stride_1(),
b.data(), b.stride_0()); Updated #if KOKKOS_VERSION >= 40599
KokkosBatched::SerialTrsv<KokkosBatched::Uplo::Upper, KokkosBatched::Trans::NoTranspose, KokkosBatched::Diag::NonUnit, KokkosBatched::Algo::Trsv::Unblocked>::invoke(1.0, A, b);
#else
KokkosBatched::SerialTrsvInternalUpper<KokkosBatched::Algo::Trsv::Unblocked>::invoke(false,
A.extent(0),
1.0,
A.data(), A.stride_0(), A.stride_1(),
b.data(), b.stride_0());
#endif |
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Address issue kokkos/kokkos-kernels#2456 (comment) Co-authored-by: Yuuichi Asahi <[email protected]> Signed-off-by: Nathan Ellingwood <[email protected]>
Thanks @yasahi-hpc , that helped make some progress. There is now a new bump:
From the error message it looks like the updates constrained the algorithm to View, if so is it possible to relax the static_assert check to allow use of DynRankView as well? Unfortunately in this case, the rank check would need to be at runtime |
We might want to update our unit tests to cover these things a bit better, that would avoid all the back and force and improve the library robustness. |
I have relaxed the |
Description:
Nightly builds of Trilinos fail to compile some packages (ifpack2, intrepid2) with output:
Intrepid2:
Ifpack2:
These errors began following merge of #2452 .
Trilinos builds enable deprecated code, I'm not sure if that indicates an inadvertent compatibility change that should be addressed, or if changes should be made in Trilinos itself (which will be necessary regardless, I suppose)
Reproducer (blake all queue):
The text was updated successfully, but these errors were encountered: