Rename distributed::Matrix non_local_matrix to ghost _matrix#2017
Open
pratikvn wants to merge 1 commit into
Open
Rename distributed::Matrix non_local_matrix to ghost _matrix#2017pratikvn wants to merge 1 commit into
pratikvn wants to merge 1 commit into
Conversation
ginkgo-bot
added
reg:testing
Member
Author
|
format! |
pratikvn
force-pushed
the
rename/dist-local-diag
branch
from
685fa10 to
38a1edf
Compare
Member
Author
|
format! |
pratikvn
changed the title
MarcelKoch
requested changes
May 11, 2026
Member
MarcelKoch
left a comment
MarcelKoch
left a comment
There was a problem hiding this comment.
Only smaller nits.
I would also suggest to have another PR to use the same naming scheme for the index map.
Comment on lines
67
to
72
| * provided as a list of tuples for each part | ||
| * of the row partition. Each tuple contains | ||
| * the size of the non-local matrix, a list of | ||
| * the size of the ghost matrix, a list of | ||
| * row indices (mapped to local indexing), a | ||
| * list of column indices (NOT mapped to local | ||
| * indexing), and a list of values. |
Comment on lines
+424
to
+425
| * this process — i.e., the columns borrowed (as ghost values) from other | ||
| * ranks during SpMV. |
Member
There was a problem hiding this comment.
This explanation seems confusing to me. Columns are not communicated during the spmv
Comment on lines
+176
to
+177
| * entries from columns that are not owned by the process (the ghost columns | ||
| * borrowed from neighbors). The ghost matrix is stored in a compressed |
Member
There was a problem hiding this comment.
Same here. The columns are not borrowed from neighbors, they only exist on this rank.
pratikvn
force-pushed
the
rename/dist-local-diag
branch
from
2976db4 to
ca8b467
Compare
yhmtsai
reviewed
May 12, 2026
Member
yhmtsai
left a comment
yhmtsai
left a comment
There was a problem hiding this comment.
LGTM. is off-diag or offdiag more often used?
| DEFINE_string(non_local_formats, "csr", | ||
| "A comma-separated list of formats for the non-local matrix to " | ||
| DEFINE_string(off_diag_formats, "csr", | ||
| "A comma-separated list of formats for the off-diag matrix to " |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR renames the distributed Matrix
non_local_matrixtoghost_matrix. It deprecates the previousnon_localnames.