-
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
BSR format support in block Gauss-Seidel #1232
BSR format support in block Gauss-Seidel #1232
Conversation
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
8282092
to
3b1f2db
Compare
@lucbv @srajama1 @brian-kelley @uhetmaniuk With the description for this PR and committed changes (selected scenario fully implemented - from user interface down to kernels, with passing unit test) I tried to illustrate proposed approach for BSR format support in GS kernel. Please let me know if such design makes sense. I'll appreciate any comments and suggestions.
|
dcbae04
to
a5bb9a0
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
6df5802
to
802d643
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
@brian-kelley Do you want to review this first for the design? |
802d643
to
caef435
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
caef435
to
df93cf3
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
b68eb95
to
5e7b27b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some comments for clean-up and a question regarding testing otherwise this looks fine so I will add the pre-test flag.
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; This inspection will remain valid until a new commit to source branch is performed. |
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: MikolajZuzek |
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 # 138 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA10_Tpls_CUDA10_LayoutRight # 135 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720 # 793 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720_Light_LayoutRight # 440 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_GCC720 # 784 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_INTEL18 # 772 (click to expand)
Console Output (last 100 lines) : KokkosKernels_PullRequest_CLANG1001 # 177 (click to expand)
|
6bafd28
to
d8a0253
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
36158f3
to
d460b59
Compare
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; This inspection will remain valid until a new commit to source branch is performed. |
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: MikolajZuzek |
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... |
3 similar comments
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@lucbv @brian-kelley Thank you for all your comments! Would you see anything for me to add or fix on this PR ? |
@MikolajZuzek No, thanks, it looks good. I just approved. |
Cool, then I am merging this. |
Contents
PointGaussSeidel
- numeric and apply;Get_Matrix_Diagonals
;Team_PSGS
:BlockTag
andBigBlockTag
operators;BlockCRS
andBSR
) through the tests;Implementation
Gauss-Seidel kernel already supports sparse block computation based on block CRS format, which differs from BSR only in the value ordering in
values
view (rows
andentries
views are identical for both formats as they index whole blocks in the same way):Because the difference lays only in the indexing method applied to values vector, it's sufficient to make that indexing method a plugin to GS kernel to have it support both formats. The implementation approach proposed on this PR has
SparseMatrixFormat
tag (BlockCRS
orBSR
) provided to GS functions together withrows
,entries
andvalues
sparse vectors. Based on that tag, GS internally creates and usesMatrixRowIndex<Format>
to access sparse values. Format tag can be easily deduced from actual matrix type withMatrixTraits<MatrixType>::format
as in the unit test.Alternatives
rows
,entries
andvalues
vectors accompanied with matrix format (asBlockCRS
orBSR
tag), we could consider passing through Gauss-Seidel interfaces aCrsMatrix
(BlockCrsMatrix
?) orBsrMatrix
handle, which binds all that necessary information together.MatrixRowIndex<Format>
could be defined in respective matrix headers instead of KokkosKernels_SparseUtils.hpp, like row views androw()
methods inCrsMatrix
andBsrMatrix
.MatrixRowIndex<...>
) could be provided to GS kernel directly instead ofSparseMatrixFormat
tag.