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

[Feature] FindEdge/FindEdges for Immutable Graph #404

Merged
merged 26 commits into from
Feb 27, 2019
Merged

Conversation

yzh119
Copy link
Member

@yzh119 yzh119 commented Feb 21, 2019

Description

FindEdge(s) and EdgeSubgraph have not been implemented in Immutable Graph. #369

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [Model], [Doc], [Feature]])
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented
  • To the my best knowledge, examples are either not affected by this change,
    or have been fixed to be compatible with this change
  • Related issue is referred in this PR

Changes

  • Add an EdgeList structure in ImmutableGraph, and a shared pointer attribute edge_list_; to avoid unnecessary memory cost, edge_list_ is created when it's required.
  • Complete EdgeSubgraph
    • Still lacks test coverage for EdgeSubgraph, but I was too lazy and I would finish it soon.

@yzh119 yzh119 requested a review from zheng-da February 21, 2019 13:33
@yzh119
Copy link
Member Author

yzh119 commented Feb 21, 2019

Works fine with Transformer.

@jermainewang jermainewang self-requested a review February 21, 2019 16:48
@yzh119
Copy link
Member Author

yzh119 commented Feb 25, 2019

@zheng-da EdgeSubgraph finished.

include/dgl/immutable_graph.h Outdated Show resolved Hide resolved
src/graph/immutable_graph.cc Outdated Show resolved Hide resolved
src/graph/immutable_graph.cc Outdated Show resolved Hide resolved
src/graph/immutable_graph.cc Outdated Show resolved Hide resolved
tests/compute/test_graph.py Show resolved Hide resolved
@jermainewang jermainewang mentioned this pull request Feb 26, 2019
26 tasks
std::vector<int64_t> *indptr,
std::vector<dgl_id_t> *indices,
std::vector<dgl_id_t> *edge_ids,
bool in_csr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make the input arguments const reference? input pointers usually mean the vectors will be modified by the method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@zheng-da
Copy link
Collaborator

everything else looks good.

@yzh119 yzh119 merged commit 724aa0c into dmlc:master Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants