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: Why remove instead of deprecate getLocalRowView? #9082

Closed
tcfisher opened this issue May 3, 2021 · 5 comments
Closed

Tpetra: Why remove instead of deprecate getLocalRowView? #9082

tcfisher opened this issue May 3, 2021 · 5 comments
Labels
client: SPARC Issues related to or needed more specifically by the ATDM SPARC code pkg: Tpetra type: question

Comments

@tcfisher
Copy link
Contributor

tcfisher commented May 3, 2021

Question

@trilinos/tpetra
@kddevin
@ccober6

The commit, 34a0760, has broken SPARC because SPARC uses getLocalRowView from TpetraBlockVector and TpetraBlockCrs. A lot of code was moved to deprecated when new interfaces were added. Why wasn't this made deprecated instead of removed?

What is the correct translation to transition from getLocalRowView to getBlock?

Thanks,

Travis

@jhux2
Copy link
Member

jhux2 commented May 3, 2021

@trilinos/tpetra

@jhux2 jhux2 added pkg: Tpetra client: SPARC Issues related to or needed more specifically by the ATDM SPARC code labels May 3, 2021
@kddevin
Copy link
Contributor

kddevin commented May 3, 2021

Removing without deprecation was likely a mistake. I'll restore it this afternoon unless there is a serious problem that led to the removal. Thanks for the report; sorry for the inconvenience.
@tcfisher

@kddevin
Copy link
Contributor

kddevin commented May 3, 2021

@tcfisher #9084 restores the missing functions. Note, however, that these functions will be deprecated.
Instead of saying

scalar *x;
mv.getLocalRowView(row, col, x);

you should do

auto x = mv.getLocalBlockHost(row, col, Access::ReadOnly);

or

auto x = mv.getLocalBlockHost(row, col, Access::ReadWrite);

The access tag says how you intend to use the returned view -- as read-only (const) data or read-write (non-const) data.

Deprecated function getLocalBlock, getLocalRowView and getGlobalRowViews were returning host data. Please let me know if you need device versions of getLocalBlock. Or if you have suggestions for better block interfaces, let us know.

Note that similar changes will happen for BlockCrsMatrix. @kyungjoo-kim is working on BlockCrsMatrix now.

@tcfisher
Copy link
Contributor Author

tcfisher commented May 3, 2021

Thanks @kddevin, we will try to transition to the new interface soon. We were only using this on the host.

trilinos-autotester added a commit that referenced this issue May 5, 2021
Automatically Merged using Trilinos Pull Request AutoTester
PR Title: Tpetra:  restore deprecated BlockMultiVector and BlockVector functions for #9082
PR Author: kddevin
@kddevin
Copy link
Contributor

kddevin commented May 22, 2021

Fixed by #9084

@kddevin kddevin closed this as completed May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client: SPARC Issues related to or needed more specifically by the ATDM SPARC code pkg: Tpetra type: question
Projects
None yet
Development

No branches or pull requests

3 participants