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

Make a deep-copy of the Tpetra::Vector in the assignment operator. #16558

Merged
merged 2 commits into from Jan 30, 2024

Conversation

kinnewig
Copy link
Contributor

Follow up of the PR #16545.

In the assignment operator, we must perform a deep-copy of the vector instead of a shallow copy.
This should fix the assignment operator.

masterleinad
masterleinad previously approved these changes Jan 29, 2024
@masterleinad
Copy link
Member

/rebuild

include/deal.II/lac/trilinos_tpetra_vector.templates.h Outdated Show resolved Hide resolved
@kronbichler
Copy link
Member

Nice, that was a good suggestion @masterleinad!

Comment on lines +293 to +300
# if DEAL_II_TRILINOS_VERSION_GTE(13, 2, 0)
auto source_vector_2d =
V.vector->template getLocalView<Kokkos::HostSpace>(
Tpetra::Access::ReadOnly);
# else
auto source_vector_2d =
V.vector->template getLocalView<Kokkos::HostSpace>();
# endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are now using deep_copy, is there a good reason to extract the host view instead of the device view? In case of a device memory space (which we don't support yet), the approach here seems to make unnecessary copies between host and device since all operations are explicitly done on the host.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just get this in and we can look at this somewhere else.

@bangerth bangerth merged commit d68d8b4 into dealii:master Jan 30, 2024
14 of 15 checks passed
@kinnewig kinnewig deleted the copy-assignment branch February 1, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants