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

lac/trilinos_tpetra...h: Fix compilation on Ubuntu 20.04 #15612

Merged
merged 1 commit into from Jul 3, 2023

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Jul 3, 2023

Last part to really: Fixes #15580

@tamiko tamiko added the Bug label Jul 3, 2023
@tamiko tamiko added this to the Developer workshop 2023 milestone Jul 3, 2023
@tamiko tamiko requested a review from drwells July 3, 2023 04:35
Comment on lines +135 to +144
/*
* For Trilinos older than 13.2 we would normally have to call
* vector.template sync<Kokkos::HostSpace>() at this place in order
* to sync between memory spaces. This is necessary for GPU support.
* Unfortunately, we are in a const context here and cannot call to
* sync() (which is a non-const member function).
*
* Let us choose to simply ignore this problem for such an old
* Trilinos version.
*/
Copy link
Member

Choose a reason for hiding this comment

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

I think this is OK but we could also check via need_sync_host(https://docs.trilinos.org/dev/packages/tpetra/doc/html/classTpetra_1_1MultiVector.html#a5af96c2c9c0212e16b88a97d3ab7a284) if we are running into issues.

Copy link
Member

Choose a reason for hiding this comment

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

You can always do a const cast. As long as the function is "semantically const", that should be ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed and agreed to keep this PR as is for the time being. We can revisit if someone ever ends up using an old Trilinos version with GPUs.

Copy link
Member

@drwells drwells left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this, but I think someone more familiar with Trilinos should have the final say on this PR.

@tamiko
Copy link
Member Author

tamiko commented Jul 3, 2023

/rebuild

@tamiko tamiko merged commit 68614f5 into dealii:master Jul 3, 2023
12 of 14 checks passed
@tamiko tamiko deleted the fix_compilation_with_trilinos_12.14 branch July 7, 2023 00:27
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.

Regression tester regressed on revision 434975
4 participants