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

[Example] Dynamic Graph CNN on Point Cloud #789

Merged
merged 15 commits into from
Aug 28, 2019
Merged

Conversation

BarclayII
Copy link
Collaborator

Description

This PR implements the Dynamic Graph CNN paper, as well as the EdgeConv module and DynamicEdgeConv module in #748 . Namely, it provides

  • EdgeConv in the said paper.
  • NearestNeighborGraph: A module that constructs a graph from a feature matrix based on nearest neighbors.
  • (Alternating between NearestNeighborGraph and EdgeConv yields DynamicEdgeConv.)

Hopefully this could be a starting point for issue #719.

This runs on ModelNet40 dataset and got a best validation accuracy of 93.5% and test accuracy of 91.8%.

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

@yzh119
Copy link
Member

yzh119 commented Aug 26, 2019

@BarclayII , could you please move the EdgeConv module to dgl.nn.*.conv? Plus, I recommend putting NearestNeighborGraph to dgl.transforms, thanks!

@BarclayII
Copy link
Collaborator Author

@BarclayII , could you please move the EdgeConv module to dgl.nn.*.conv? Plus, I recommend putting NearestNeighborGraph to dgl.transforms, thanks!

I'm actually thinking what the form of EdgeConv and NearestNeighborGraph should be. In particular, whether we want to support point cloud with different number of points.

@BarclayII
Copy link
Collaborator Author

Temporarily closing to save CI as I'm migrating code to dgl.nn.

@BarclayII BarclayII closed this Aug 27, 2019
@BarclayII BarclayII reopened this Aug 27, 2019
@BarclayII BarclayII closed this Aug 27, 2019
@BarclayII BarclayII reopened this Aug 27, 2019
Copy link
Member

@yzh119 yzh119 left a comment

Choose a reason for hiding this comment

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

Great work! Go ahead and merge it.

@BarclayII BarclayII merged commit dc19cd5 into dmlc:master Aug 28, 2019
@BarclayII BarclayII deleted the edgeconv branch August 28, 2019 01:22
@jermainewang
Copy link
Member

@BarclayII could you rename nearest_neighbor_graph to just knn_graph and segmented_knn_graph? They are much shorter.

@BarclayII BarclayII restored the edgeconv branch August 28, 2019 03:47
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.

3 participants