-
Notifications
You must be signed in to change notification settings - Fork 578
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
Belos & Amesos2: changes following Tpetra_UVM_Removal branch #9220
Conversation
@hkthorn ?? |
@jennloe @jhux2 @ndellingwood I have no obvious objections to the Belos changes, but many of those changes have more to do with Tpetra than Belos. I would rather these changes be approved by someone more informed of the Tpetra UVM-removal work. |
As a Tpetroid, the Belos changes look fine to me! |
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: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_8.3.0 # 4652 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_serial # 2182 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_debug # 2663 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_intel_17.0.1 # 9974 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105 # 1388 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105_uvm_off # 385 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_clang_10.0.0 # 2753 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_python_3 # 5322 (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: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Using Repos:
Pull Request Author: iyamazaki |
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: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
Jenkins Parameters
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_8.3.0 # 4655 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_serial # 2185 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_gcc_7.2.0_debug # 2666 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_intel_17.0.1 # 9976 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105 # 1390 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_cuda_10.1.105_uvm_off # 387 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_clang_10.0.0 # 2755 (click to expand)
Console Output (last 100 lines) : Trilinos_pullrequest_python_3 # 5324 (click to expand)
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Failure: Timed out waiting for job Trilinos_pullrequest_intel_17.0.1 to start: Total Wait = 603
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
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: Trilinos_pullrequest_gcc_8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_serial
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0_debug
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_10.1.105_uvm_off
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_clang_10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_python_3
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... |
@@ -208,10 +202,10 @@ namespace Amesos2 { | |||
const size_t lclNumRows = redist_mv.getLocalLength(); | |||
for (size_t j = 0; j < redist_mv.getNumVectors(); ++j) { | |||
auto av_j = av(lda*j, lclNumRows); | |||
auto X_lcl_j_2d = redist_mv.getLocalViewHost(); | |||
auto X_lcl_j_2d = redist_mv.getLocalViewHost(Tpetra::Access::ReadOnly); |
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 don't think X_lcl_j_2d
is necessary here since you already have contig_local_view_2d
above.
auto X_lcl_j_1d = Kokkos::subview (X_lcl_j_2d, Kokkos::ALL (), j); | ||
|
||
using val_type = typename decltype( X_lcl_j_1d )::value_type; | ||
using val_type = typename std::remove_const<typename decltype( X_lcl_j_1d )::value_type>::type; |
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.
Could you use typename multivec_t::impl_scalar_type
instead for readability?
auto X_lcl_j_1d = Kokkos::subview (X_lcl_j_2d, Kokkos::ALL (), j); | ||
|
||
using val_type = typename decltype( X_lcl_j_1d )::value_type; | ||
using val_type = typename std::remove_const<typename decltype( X_lcl_j_1d )::value_type>::type; |
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.
Same as above - could use multivec_t::impl_scalar_type
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.
Amesos2 changes look fine (a few remnant commented lines can be cleaned up later) and testing passes, thanks for the hard work @iyamazaki !
Thank you for the comments, @brian-kelley. I have the changes, but I am thinking to push them as a separate PR (since auto-tester seems to be struggling right now). Or, I can create an issue with a list of changes if @ndellingwood also has some inputs. What do you think? |
@iyamazaki That sounds fine. I just approved this, but my comments from the first review are still up there. |
@prwolfe @allevin Is it possible that there are just a bunch of jobs in this tester's swim lane and we need to wait a bit more for it to cycle around or could something else be blocking it? @csiefer2 Paul or Aaron may have more insight into the delay than I do... but in the past my experience has generally been that the tester needs to cycle around and do its check of the PR to set the final flags. Still, you approved this about 26 hours ago which seems long... but builds also take a long time too so if there's other stuff pending.... 🤷🏻♂️ |
@william76 - we are currently struggling with the change in the proxy system and it's implications when run through python. Basically I can get the autotester to talk to GitHub or to talk with Jenkins, but not both as we need. |
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ ndellingwood brian-kelley csiefer2 ]! |
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
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.
Not sure why Amesos2 is still showing as locked, approving to see if that fixes it.
@trilinos/tpetra, @trilinos/belos, and @trilinos/amesos2
Motivation
This PR contains changes needed for Belos & Amesos2, following up the recent changes in Tpetra to remove UVM dependencies (PR#9038)
Related Issues
Closes Issue#8761
Testing
Tested on Vortex with/without UVM. For Amesos2, we tested with KLU2 and SuperLU, using MPI enabled.