-
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
[Bugfix] Fix multiple send recv #320
Conversation
python/dgl/utils.py
Outdated
# XXX: assuming tensor supports index using slices | ||
# cannot call tousertensor because for slice type it would | ||
# further call tonumpy, which overwrites _pydata and causes | ||
# future bugs... |
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.
What does this mean?
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.
Originally when I implemented get_items and set_items, I write
if isintance(index, Index):
index = index.tousertensor()
And I just found this is buggy, because, if index._pydata is a slice, index.tousertensor will can tonumpy, which overwrites _pydata. As a result, index._pydata becomes a numpy.ndarray... And this can potentially cause bugs
python/dgl/runtime/scheduler.py
Outdated
if apply_func is not None: | ||
schedule_apply_nodes(graph, recv_nodes, apply_func, inplace) | ||
else: | ||
src, dst, _ = graph._graph.find_edges(eid) |
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.
lingfan: Mistakenly removed your comment...
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.
You are right about using mask on src, dst to avoid find_edge
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.
LTGM. Minor comments.
Description
This PR fixes #75 and other small bugs (see below) and supports multiple recv after send.
Previously, each recv call clears message graph so following recv won't see any message. This PR fix the bug by removing message graph and put message in edge space (but still in a separate message frame). Now message frame has the same size as edge frame and whether there is a message on an edge is indicated by a special "boolean" index in DGLGraph (graph._msg_index).
Now users can:
Checklist
or have been fixed to be compatible with this change
Changes