-
Notifications
You must be signed in to change notification settings - Fork 576
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
kokkos: view_copy fix #2863
kokkos: view_copy fix #2863
Conversation
view_copy fix in Kokkos deep_copy and unit tests added. Debugging and fix with @crtrott and @dsunder. Issue exposed by failing Panzer examples reported in #2827. This PR is a patch created from kokkos/kokkos#1653. Changes to be committed: modified: ../src/Kokkos_CopyViews.hpp modified: CMakeLists.txt modified: Makefile new file: TestViewCopy.hpp new file: cuda/TestCudaHostPinned_ViewCopy.cpp new file: cuda/TestCudaUVM_ViewCopy.cpp new file: rocm/TestROCmHostPinned_ViewCopy.cpp
PR created from a patch of PR kokkos/kokkos#1653. |
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_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
atdm cuda-dbg build going now, will post results once it finishes. |
Panzer Example test results - spot check from failures reported in #2827 CurlLaplacianExample:
MixedPoissonExample:
PoissonExample:
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 4 Hrs. 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_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
autotesting failed due to |
@ibaned, I saw this test I don't understand how these Kokkos changes could impact this test but it is possible since the TeuchosCore subpackage does have an optional dependency on the KokkosCore subpackage. This is not the only time this test has failed in some builds in all recorded history (about 6 months worth now on testing-vm.sandia.gov/cdash/). If you look at the query: you can see that this test has failed 4 other times. But all 4 of those other failures look to be configuration or other system issues and therefore do not look to be a code problem. Note that this is a new Intel auto PR build and it may still have some problems. (See #2864). Let's run the auto PR tests again and see what happens. |
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.
@ndellingwood, the code changes look reasonable to me, but I am not sure that I am qualified to review this code. But if this passes the auto PR builds and passes the CUDA tests for the ATDM builds, then I am good with this.
Wait, I know why the test which shows:
The problem is that the output on rank 0 got jumbled with the startup banner on rank 1. This was just very bad luck. I will submit a PR to disable the startup banner for this test. |
Thanks @bartlettroscoe ! |
…inos#2863) I added --teuchos-suppress-startup-banner to the multi-MPI process tests that grep the output for pass/fail. Otherwise, the ouptut on proc 0 can get jumbled with the statup banner output on other procs. This happened, for example, in the PR testing for PR trilinos#2863. This will stop that from happening in the future.
FYI: PR #2865 will avoid that Teuchos test from tripping up any future auto PR builds. But it was a rare random failure that should not occur again in this PR. I set the |
Status Flag 'Pull Request AutoTester' - User Requested Retest - Resetting Testing Status |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
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_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ bartlettroscoe ]! |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
This PR is ready to merge. Can we pull the trigger and merge this? |
I added --teuchos-suppress-startup-banner to the multi-MPI process tests that grep the output for pass/fail. Otherwise, the ouptut on proc 0 can get jumbled with the statup banner output on other procs. This happened, for example, in the PR testing for PR #2863. This will stop that from happening in the future.
view_copy fix in Kokkos deep_copy and unit tests added.
Debugging and fix with @crtrott and @dsunder.
Issue exposed by failing Panzer examples reported in #2827.
This PR is a patch created from kokkos/kokkos#1653.
Changes to be committed:
modified: ../src/Kokkos_CopyViews.hpp
modified: CMakeLists.txt
modified: Makefile
new file: TestViewCopy.hpp
new file: cuda/TestCudaHostPinned_ViewCopy.cpp
new file: cuda/TestCudaUVM_ViewCopy.cpp
new file: rocm/TestROCmHostPinned_ViewCopy.cpp