Add batched GESV #1384
Merged
Add batched GESV #1384
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_Tpls_CUDA10_Tpls_CUDA10_LayoutRight_GCC720_Light_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Using Repos:
Pull Request Author: kliegeois |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_Tpls_CUDA10_Tpls_CUDA10_LayoutRight_GCC720_Light_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
1 similar comment
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
lucbv
requested changes
Apr 20, 2022
example/CMakeLists.txt
Outdated
| @@ -7,3 +7,4 @@ KOKKOSKERNELS_INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/../test_common) | |||
| #ADD_SUBDIRECTORY(graph) | |||
| ADD_SUBDIRECTORY(wiki) | |||
| ADD_SUBDIRECTORY(gmres) | |||
| ADD_SUBDIRECTORY(static_pivoting) | |||
There was a problem hiding this comment.
I'm probably being just too picky but how about renaming this directory batched solve and then naming the example static pivoting? This way we could add further tests for other batched linear solver/factorization down the road.
| //@HEADER | ||
|
|
||
| template <class XType> | ||
| void write2DArrayToMM(std::string name, const XType x) { |
There was a problem hiding this comment.
This should be put in src/common/KokkosKernels_IOUtils.hpp there are already a few matrix market utilities there.
| } | ||
|
|
||
| template <class XType> | ||
| void write3DArrayToMM(std::string name, const XType x) { |
| } | ||
|
|
||
| template <typename MatrixViewType, typename VectorViewType> | ||
| void create_saddle_point_matrices(const MatrixViewType &A, |
There was a problem hiding this comment.
You might want to add a little comment to explain what n_2 is?
And maybe just explain the structure of the saddle point problem for future users?
| using value_type = typename MatrixType1::non_const_value_type; | ||
| const int n = A.extent(0); | ||
|
|
||
| for (int i = 0; i < n; ++i) { |
There was a problem hiding this comment.
Add a comment to explain what this loop does
| const int n = A.extent(0); | ||
|
|
||
| for (int i = 0; i < n; ++i) { | ||
| D2(i) = 0.; |
There was a problem hiding this comment.
I have a feeling that using Kokkos::ArithTraits<scalar_type>::zero() instead of 0. will avoid maintenance issues down the road...
| const VectorType2 PDY, const VectorType2 D2, const VectorType2 tmp_v_1, | ||
| const VectorType2 tmp_v_2) { | ||
| using value_type = typename MatrixType1::non_const_value_type; | ||
| const int n = A.extent(0); |
There was a problem hiding this comment.
Does View::extent() always return int, I thought on some architectures it might be size_t?
| D2(i) = 1. / D2(i); | ||
| } | ||
|
|
||
| for (int i = 0; i < n; ++i) { |
There was a problem hiding this comment.
Add a quick comment like // scaling columns of A by their largest absolute value
Also inverting these two loops might keep D2(j) in a register instead of streaming the values of D2 from cache. This might already happen by compiler optimization?
| /// | ||
| /// Solve A_l x_l = b_l for all l = 0, ..., N | ||
| /// using a batched LU decomposition, 2 batched triangular solves, and a batched | ||
| /// static pivoting. |
There was a problem hiding this comment.
You should explain a little what pivoting strategy is used.
| } | ||
|
|
||
| for (int i = 0; i < n; ++i) { | ||
| value_type D1_i = 0.; |
There was a problem hiding this comment.
Could be allocated outside of the loop and simply re-initialized to zero inside.
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
|
Thanks @lucbv for your review. I am updating the PR! |
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Using Repos:
Pull Request Author: kliegeois |
srajama1
requested changes
Apr 20, 2022
| @@ -162,16 +167,16 @@ int main(int /*argc*/, char ** /*argv[]*/) { | |||
| Kokkos::deep_copy(A2, A); | |||
| Kokkos::deep_copy(Y2, Y); | |||
|
|
|||
| write3DArrayToMM("A.mm", A); | |||
| write2DArrayToMM("Y.mm", Y); | |||
| KokkosKernels::Impl::kk_write_3Dview_to_file(A, "A.txt"); | |||
There was a problem hiding this comment.
Do we always write to a file in the directory wherever the executable is run from? This is not common practice for any of our other tests. Do we need to write it or just pushing it to std out is enough?
There was a problem hiding this comment.
It is only for an example, not for a (unit) test, I can remove that if you prefer.
There was a problem hiding this comment.
But yes, it writes to a file in the directory where you run the example executable, I did so mostly for users to check the computation if they want.
There was a problem hiding this comment.
It is ok. We typically don't do it, but that doesn't mean it is wrong. In Trilinos land, we avoid it because it is parallel file write. It shouldn't be a problem in KK.
srajama1
approved these changes
Apr 20, 2022
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740 # 159 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight # 151 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720 # 962 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720_Light_LayoutRight # 606 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_GCC720 # 950 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_INTEL18 # 937 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_CLANG1001 # 342 (click to expand) |
kliegeois
added
AT: RETEST
kliegeois
force-pushed
the
static_pivoting_PR
branch
from
0c7689d to
c65915d
Compare
|
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Using Repos:
Pull Request Author: kliegeois |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740 # 162 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight # 154 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720 # 965 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720_Light_LayoutRight # 609 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_GCC720 # 953 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_INTEL18 # 940 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_CLANG1001 # 345 (click to expand) |
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
Using Repos:
Pull Request Author: kliegeois |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9_GCC720_Light_Tpls_GCC720_GCC740
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_GCC720_Light_LayoutRight
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_INTEL18
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_CLANG1001
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
lucbv
approved these changes
Apr 21, 2022
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ lucbv ]! |
|
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the batched GESV as requested in #1349.
This PR includes a serial GESV, a team GESV, and a team vector GESV.
All those implementations are tested in unit tests and an example is provided to solve batched saddle point systems.
The strategy used to do the pivoting relies only on finding the maximal value (and the corresponding index) along a row or a column. This is not the optimal pivots but their computations can be evaluated efficiently using thread and vector parallel for loops.
The tests passed on my workstation and on Weaver.