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: Add CrsGraph & CrsMatrix constructors that take 4 Maps and a local graph / matrix #1536

Closed
mhoemmen opened this issue Jul 26, 2017 · 5 comments
Assignees

Comments

@mhoemmen
Copy link
Contributor

@trilinos/tpetra Requested by @aprokop . See #1525.

@aprokop , at the point you would want to call these constructors, would you also happen to have the Import and Export objects if they are needed? If so, the constructor should take them, to avoid reconstruction.

@aprokop
Copy link
Contributor

aprokop commented Jul 26, 2017

@mhoemmen No, I do not construct import/export. The only thing I have are 4 maps: 2 permuted ones (row and column) and 2 original ones (domain, range). I would hope Tpetra would take care of everything else.

Do you want me to do a PR (WIP) for this issue? I would simply cherry-pick some commits from #1525.

@mhoemmen
Copy link
Contributor Author

@aprokop wrote:

Do you want me to do a PR (WIP) for this issue?

Could you please? Thanks :-) ! It should be easy to add a test: Come up with an example with four distinct Maps, then build two matrices, one with the old (rowMap, colMap, local matrix) constructor and one with the new constructor. Then, fill complete both, and test sameness (not pointer equality, we don't necessarily promise that) of Maps.

@aprokop
Copy link
Contributor

aprokop commented Jul 27, 2017

@mhoemmen I cannot find a unit test for the existing constructor (2 maps + local matrix). Is there one? My hope was to copy-paste that one.

@mhoemmen
Copy link
Contributor Author

The closest thing would be Lesson07, but I think it exercises the 2 Maps + three arrays constructor for CrsMatrix, not the 2 Maps + local_matrix_type constructor.

@aprokop
Copy link
Contributor

aprokop commented Aug 4, 2017

Pushed to upstream. Thanks @mhoemmen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants