trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.22k stars 568 forks source link

Tpetra::MultiVector: Fix "DualView" (better: "dual view") semantics for subviews #364

Open mhoemmen opened 8 years ago

mhoemmen commented 8 years ago

@trilinos/tpetra

Currently, all subviews (row or column) of a Tpetra::MultiVector share the same modify() / sync() flags. Consider the following case: X is the original MultiVector, and Y and Z are disjoint subviews of X. If Y syncs, currently that will cause all of X to sync, and thus force a (possibly incorrect) sync of Z as well.

The reason for this is that Tpetra::MultiVector lets Kokkos::DualView own the modify() flags. To fix this, Tpetra needs to control those flags and own sync'ing.

Fixing this would also give Tpetra the flexibility to do on-demand allocation, and even deallocate device / HBM memory when not in use (since it may be a scarce resource). However, we would need to get rid of the Tpetra::MultiVector::getDualView method (and thus no longer expose the Kokkos::DualView).

mhoemmen commented 8 years ago

We could even fix getDualView(), if we change it so it returns a new Kokkos::DualView with the Tpetra::MultiVector's modified flags. The only exception is views of noncontiguous columns, and the only way we could fix that would be for getDualView() to return either a Kokkos::DualView with the usual layout, or something with a special noncontiguously strided layout. Those two DualViews would have different types (due to their different layouts), so we would have to wrap them in some kind of proxy object.... It gets a bit complicated.

mhoemmen commented 8 years ago

Just to clarify my second comment above: If Tpetra::MultiVector owns dual view semantics (that is, if it doesn't use Kokkos::DualView, but instead keeps the Views and "modified" flags itself), and if users never call getDualView(), then we can fix this issue completely, even for Tpetra::MultiVector views of noncontiguous column sets.

If users call getDualView(), it would be possible for users to use the returned Kokkos::DualView to break dual view semantics for the aforementioned views of noncontiguous column sets. We could treat getDualView() as an expert-mode method that we plan to deprecate and remove anyway. That seems like a good idea -- the Kokkos::DualView object isn't really what they would ever want anyway.

github-actions[bot] commented 3 years ago

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity. If you would like to keep this issue open please add a comment and remove the MARKED_FOR_CLOSURE label. If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.

csiefer2 commented 2 years ago

One might be able to modify WrappedDualView to do exactly that since that's a natural place to do it. I guess the question to ask is "How often does this pattern actually occur in our applications?"