Skip to content
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: Don't test need_sync_[host|device] #13833

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

masterleinad
Copy link
Contributor

@trilinos/tpetra

Motivation

kokkos/kokkos#7775 proposes to skip modify and sync if DualView only references one View, i.e., if the memory space is host-accessible. This changes the behavior of DualView::need_sync_[host|device] and thus
Tpetra::MultiVector::need_sync_[host|device] which just calls the respective DualView function, see

template <class Scalar, class LocalOrdinal, class GlobalOrdinal, class Node>
bool
MultiVector<Scalar, LocalOrdinal, GlobalOrdinal, Node>::
need_sync_host () const {
return view_.need_sync_host ();
}
template <class Scalar, class LocalOrdinal, class GlobalOrdinal, class Node>
bool
MultiVector<Scalar, LocalOrdinal, GlobalOrdinal, Node>::
need_sync_device () const {
return view_.need_sync_device ();
}
. One of the Tpetra unit tests checks this behavior explicitly and therefore breaks with kokkos/kokkos#7775.
This pull request proposes to just remove this particular check since the MultiVector function only forwards anyway.
An alternative would be to use

constexpr bool needsSyncPath = !Kokkos::SpaceAccessibility<Kokkos::HostSpace, typename device_t::memory_space>::accessible;

once Kokkos is updated in Trilinos.

Testing

Running all Tpetra unit tests with a configuration that has Kokkos_ENABLE_CUDA_UVM=ON.

@masterleinad masterleinad requested a review from a team as a code owner February 24, 2025 21:52
@trilinos-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants