-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Misc] Provide a "return 1s instead of edge IDs" option in scipy adjacency matrix #730
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you haven't changed the elements in the returned matrix accordingly.
python/dgl/graph_index.py
Outdated
rst = _CAPI_DGLGraphGetAdj(self, transpose, fmt) | ||
if fmt == "csr": | ||
indptr = utils.toindex(rst(0)).tonumpy() | ||
indices = utils.toindex(rst(1)).tonumpy() | ||
shuffle = utils.toindex(rst(2)).tonumpy() | ||
shuffle = utils.toindex(rst(2)).tonumpy() if return_edge_ids else np.ones_like(indices) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend to rename this variable as data.
python/dgl/graph_index.py
Outdated
n = self.number_of_nodes() | ||
return scipy.sparse.csr_matrix((shuffle, indices, indptr), shape=(n, n)) | ||
elif fmt == 'coo': | ||
idx = utils.toindex(rst(0)).tonumpy() | ||
n = self.number_of_nodes() | ||
m = self.number_of_edges() | ||
row, col = np.reshape(idx, (2, m)) | ||
shuffle = np.arange(0, m) | ||
shuffle = np.arange(0, m) if return_edge_ids else np.ones_like(row) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same.
Description
Added a return_edge_id optional argument in DGLGraph.adjacency_matrix_scipy() method which defaults to True. If False, then 1's are returned.
The reason to do this is that if edge IDs are returned, then one of the nonzero entries will be actually 0, which may be confusing.
In the next major release (0.5) it will default to False to keep consistent with
adjacency_matrix
interface. Since existing code may depend on the current behavior that returns edge IDs, I added aFutureWarning
.This PR is separated from #720
Checklist
Please feel free to remove inapplicable items for your PR.
or have been fixed to be compatible with this change