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

Template TrilinosWrappers::Vector on memory space #16565

Merged

Conversation

masterleinad
Copy link
Member

For consistency with TpetraWarappers::SparseMatrix, TpetraWrappes::SparsityPattern, and LinearAlgebra::distributed::Vector, we should also template TpetraWrappers::Vector on the memory space being used.
In particular, we need matching memory spaces for the sparse matrix-vector product anyway.
This also avoids allocating a second View when compiling with a GPU backend but only using host capabilities for these classes.

@bangerth
Copy link
Member

@jpthiele @kinnewig FYI.

Comment on lines 128 to 129
template <typename Number>
template <typename Number, typename MemorySpace>
class VectorReference
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps default the argument here as well? It probably doesn't matter given that people will unlikely create objects of this type by hand, but it won't hurt.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jpthiele
Copy link
Contributor

Looks good and makes sense!

@kinnewig
Copy link
Contributor

Thank you! I was planning on doing that on my own, but I hadn't found the time yet.

@kinnewig
Copy link
Contributor

I just checked it out and ran a few more tests, everything seems to work fine!

@bangerth bangerth merged commit ccaed39 into dealii:master Jan 31, 2024
15 checks passed
@bangerth
Copy link
Member

Thank you, @masterleinad for making the change, and @kinnewig for confirming!

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