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

Allow dyn-rank-view in serial trsv #2464

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

yasahi-hpc
Copy link
Contributor

@yasahi-hpc yasahi-hpc commented Dec 17, 2024

Improves #2456

  • Allow to use Kokkos::DynRankView
  • Apply the check function to MKL wrappers
  • Unit-tests with Kokkos::DynRankView
  • Minor improvements of unit-tests to check the errors and use ValueType as a value_type of Views

@lucbv
Copy link
Contributor

lucbv commented Dec 17, 2024

@ndellingwood can you give this a review based on what you saw in Trilinos?

@ndellingwood
Copy link
Contributor

@lucbv @yasahi-hpc , thanks for the PR, I'll launch a build to test intrepid2

@ndellingwood
Copy link
Contributor

Compilation of intrepid2 now succeeds, but many tests are failing so some incompatibility still exists, this is a sample output:

Intrepid2_unit-test_Discretization_Basis_HGRAD_LINE_Cn_FEM_JACOBI_test_01_Serial_DOUBLE

12:57:28 | TEST 1: Basis creation, exceptions tests                                    |
12:57:28 ===============================================================================
12:57:28 [Intrepid2] Error in file /home/jenkins/blake-new/workspace/KokkosEco_Trilinos_Blake_LLVM1507_Serial_debug/Trilinos/packages/intrepid2/src/Discretization/Basis/Intrepid2_Basis.hpp, line 898
12:57:28             Test that evaluated to true: subcDim < 0 || subcDim >= static_cast<ordinal_type>(tagToOrdinal_.extent(0))
12:57:28             >>> ERROR (Basis::getDofOrdinal): subcDim is out of range 
12:57:28 Expected Error ----------------------------------------------------------------
12:57:28 >>> ERROR (Basis::getDofOrdinal): subcDim is out of range

@yasahi-hpc
Copy link
Contributor Author

yasahi-hpc commented Dec 18, 2024

I am not 100% sure what is wrong, but Trilinos seems to expect out-of-range access

https://github.com/trilinos/Trilinos/blob/7f913e35edaa2f6b7e2d6b67e966574db5209a79/packages/intrepid2/unit-test/Discretization/Basis/HVOL_HEX_Cn_FEM/test_01.hpp#L116-#L124

If this is the case, we need to remove checks to allow invalid accesses for these tests

Yuuichi Asahi added 2 commits December 18, 2024 10:44
Signed-off-by: Yuuichi Asahi <[email protected]>
@ndellingwood
Copy link
Contributor

Thanks @yasahi-hpc , I retested with your changes and kokkos/kokkos@71cae1c (to avoid some other issues unrelated to this PR) and the intrepid2 tests were passing in (clang/15, Serial backend tested)

@ndellingwood ndellingwood merged commit 07de262 into kokkos:develop Dec 19, 2024
18 checks passed
@yasahi-hpc
Copy link
Contributor Author

Thanks @yasahi-hpc , I retested with your changes and kokkos/kokkos@71cae1c (to avoid some other issues unrelated to this PR) and the intrepid2 tests were passing in (clang/15, Serial backend tested)

@ndellingwood That is nice. Thank you for your help.

@ndellingwood
Copy link
Contributor

@yasahi-hpc you did all the hard work, thank you for your fast responses and help!

@yasahi-hpc
Copy link
Contributor Author

@ndellingwood Thanks is mine! Hope my feature PRs do not break Trilinos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT2-CI-APPROVAL Approve CI to run at SNL bug Cleanup Code maintenance that isn't a bugfix or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants