Two-stage GS: add damping factors #921
Conversation
> damping factors for outer and inner sweeps > compact form (needs a fewer flops, may be of a lower-quality) also removed some codes, which are now not used
|
Results of spot-check on White (../scripts/cm_test_all_sandia --spot-check --arch=Power8,Pascal60 --num=1)
|
|
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_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
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... |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
|
|
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
2 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... |
|
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... |
src/common/KokkosKernels_Handle.hpp
Outdated
| void set_gs_set_num_inner_sweeps (int num_inner_sweeps) { | ||
| auto gs2 = get_twostage_gs_handle(); | ||
| gs2->setNumInnerSweeps (num_inner_sweeps); | ||
| } | ||
| // ---------------------------------------- // | ||
| // Specify damping factor of inner sweeps for two-stage Gauss-Seidel | ||
| void set_gs_set_inner_damp_factor (scalar_t_ damp_factor) { |
There was a problem hiding this comment.
nnz_scalar_t should be used inside the handle, rather than the raw template argument scalar_t_. We allow a const type for the template argument, and nnz_scalar_t removes the const.
| @@ -650,6 +701,11 @@ namespace KokkosSparse{ | |||
| values_view_t D; | |||
| crsmat_t crsmatL; | |||
| crsmat_t crsmatU; | |||
| // > complements for compact form of recurrence | |||
| // where La = A - U and Ua = A - U | |||
There was a problem hiding this comment.
Is that a typo? Maybe it should say Ua = A - L?
| if (direction == GS_FORWARD || | ||
| (direction == GS_SYMMETRIC && sweep%2 == 0)) { | ||
| // Z = (L+D)^{-1} * R | ||
| //if (sweep == 0) { |
There was a problem hiding this comment.
Can this commented-out code be removed?
| @@ -742,7 +866,7 @@ namespace KokkosSparse{ | |||
| y_value_array_type localB, // in | |||
| bool init_zero_x_vector = false, | |||
| int numIter = 1, | |||
| scalar_t omega = Kokkos::Details::ArithTraits<scalar_t>::one(), | |||
| scalar_t omega = ST::one(), | |||
There was a problem hiding this comment.
Could you add a runtime check that the omega passed in here matches gsHandle->getOuterDampFactor()? I think it's fine to do the triangular scaling by omega during setup rather than apply, but checking for that might prevent confusing behavior. I'm worried the user might thing passing it to apply() is all they need to do, since that's how MTGS works.
There was a problem hiding this comment.
Hi, @brian-kelley. The damping factor is currently used only for two-stage iteration (I had the sptrsv option mainly for debugging before the damping factor is integrated).
For using sptrsv, I am explicitly extracting the triangular part during the setup, but I don't have omega till apply, so I am assuming to be one. Then sptrsv is called for apply, so omega does not do anything. I can check omega=one at runtime during the apply with sptrsv, but what should I do if it is not omega?
There was a problem hiding this comment.
@iyamazaki I see. If this path is mainly for debugging, I think it's fine to just do
if(omega != 1.0) throw invalid_argument(...).
But if it's easy, you could make this path support arbitrary omega by adding extra BLAS calls.
There was a problem hiding this comment.
I added throw invalid_argument, thank you @brian-kelley.
There was a problem hiding this comment.
Hi, @brian-kelley, again.. Apparently, we test this option with omega=0.9.. I reset omega to be 1.0 for the unit-test to pass. I hope this is okay.
| //} | ||
| if (!two_stage) { | ||
| // ===== sparse-triangular solve ===== | ||
| // TODO: omega is not supported here (because omega*L + D is extracted in initialize_numeric, but omega is passed into apply) |
There was a problem hiding this comment.
Again, I think it's fine to deal with omega != 1.0 during numeric rather than apply, so you could just remove this TODO. It would also be good to go through the comments here like Z = (omega * L + D)^{-1} * R and either not mention omega at all (since crsmatU is already omega * L + D) or clarify by saying Z = (omega * L + D)^{-1} * R = crsmatU^{-1} * R.
|
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... |
|
Thank you, @brian-kelley !! I pushed the fixes, and am rerunning the spot-check on white. |
|
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_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
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... |
|
@iyamazaki You don't have to run the spot check manually anymore, Kokkos Kernels has an autotester like Trilinos now. |
|
Thanks for making the changes, you can merge whenever you like. |
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ brian-kelley ]! |
|
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
|
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_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
|
Thank you, @brian-kelley. I did not realize I don't need to spot-check anymore. Can you merge if this looks okay? I am not authorized to do that. |
|
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_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720 # 127 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_GCC720 # 120 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_INTEL18 # 106 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_GCC720_Light # 141 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA10 # 105 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_CUDA9 # 100 (click to expand)Console Output (last 100 lines) : KokkosKernels_PullRequest_Tpls_GCC720_GCC740 # 99 (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_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: KokkosKernels_PullRequest_GCC720
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_GCC720_Light
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA10
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_CUDA9
Jenkins Parameters
Build InformationTest Name: KokkosKernels_PullRequest_Tpls_GCC720_GCC740
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... |
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ brian-kelley ]! |
|
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - This Repo does not support Automerge |
|
Thank you, @brian-kelley. |
This PR adds new options for two-stage GS:
It also removes some codes, that are now not used.