-
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
Tpetra: Fix makeDualViewFromOwningHostView if the device View is host-accessible #13817
Tpetra: Fix makeDualViewFromOwningHostView if the device View is host-accessible #13817
Conversation
…-accessible Signed-off-by: Daniel Arndt <[email protected]>
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: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
Using Repos:
Pull Request Author: masterleinad |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
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... |
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.
lgtm
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ ndellingwood ]! |
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... |
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... |
@trilinos/tpetra
Motivation
Fixes #13778 (comment). The code preceding #13778 had
to create a
Kokkos::DualView
using a shallow copy ofhostView
as the hostView
. In case theDualView
assumes to reference only oneView
, i.e., the device memory space is host-accessible, andmodify
andskip
are no-ops, this is problematic. #13778 didn't fix this issue but made it detectable by using theDeviceView
constructor instead of assigning the hostView
directly. The check for the allocations to be the same has been implemented in kokkos/kokkos#7712.This pull request addresses the problem by not trying to preserve the device
View
in this case but to construct the DualView usinghostView
for both arguments. Note that this (implicitly) requires that device memory space is assignable from the host memory space. Otherwise, there is no way to create theDualView
from a shallow copy ofhostView
.Testing
Running all
Tpetra
tests with aKokkos
development version using theSerial
backend only and withKokkos_ENABLE_CUDA_UVM=ON
in a different configuration.