-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit f5650c0
committed
feat(MeshCNN): Add MeshCNN's Convolution Layer
This commit is the first part of a series of pull requests that we will
make to PyTorch Geometric in order add MeshCNN! MeshCNN is a complex and
important paper. As such its integration into PyG will be via a series
of self-contained PRs.
# Background
The full plan for MeshCNN is posted in the original issue here: #283
In addition, it's important to recognize that the first #2293 is quite
comprehensive and complete. The code we put here for MeshCNN will probably
look very much like the original PR made #2293. The main, difference is that
we will split up the pull requests into a series of smaller ones.
Again, this is part 1 of a comprehensive plan to add MeshCNN into
PyTorch Geometric. Please see #283
<#283>
for the comprehensive plan. It is important to understand the paper to an
extent,
*a PDF is attached to the
[issue](#283)
with a comprehensive/self-contained explanation of the paper and
how I plan to implement it.*
Please add all comments and feedback
related to the entire plan or higher level ideas.
I would love critical feedback on the plan and am happy to completely
throw it away or change it based on the feedback I get.
To _briefly_ restate the background here (again please see
[#238](#283),
MeshCNN is a classifier on triangular meshes. MeshCNN has 5 layers,
a triangular mesh to feature layer, a convolutional layer, a pooling
layer, an unpooling layers and a final "normalization" layer (to ensure a
MeshCNN is a valid probability mass function).
This pull request is the first step of the plan,
and implements the convolutional layer of MeshCNN.
# PR's Design
A good place to fully-understand `MeshCNNConv`'s design is by rendering
the docs for `torch.nn.conv.MeshCNNConv` and reading it.
The most important aspect of this pull request is ordering
we require on the `edge_index` (edge adjacency). This constraint is
documented in the class (`torch.nn.conv.MeshCNNConv`).
My questions are:
1. Should I write the core logic of MeshCNN's (non-permutation
invariant) convolution as a new `Aggregate` module?
2. MeshCNN's convolution can actually be expressed in terms of the framework of
`torch_geometric.nn.conv.RGCNConv`, where in MeshCNN there are 4
categories. Should we sub-class that instead?
3. Should we implement batch normalization as a performance optimization
here?
I have thought about these questions a lot, and if
any of these questions are "yes", my rebuttals to them are
1. MeshCNN's aggregation is not generalizable enough to warrant the
creation of a new aggregation file. That would introduce two new
files to implement `MeshCNNConv`, that is
`torch.nn.aggr.MeshCNNConv`, and the smaller wrapper
`torch.nn.conv.MeshCNNConv`. That being said, moving to
`torch.nn.aggr.MeshCNNConv` is easy to do.
2. While MeshCNN can be expressed in terms of
`torch_geometric.nn.conv.RGCNConv`, it ties us to any performance
characteristics etc that this `torch_geometric.nn.conv.RGCNConv` may
have. I think it is wrong to couple us to this framework, even if
conceptually it is important and interesting to recognize MeshCNNConv
as a case of `torch_geometric.nn.conv.RGCNConv`.
3. This can come as a future and enhancement PR after MeshCNN is more
situated into PyG. Adding this complexity now obscures the main and
lasting design decisions around our implementation of MeshCNN. These
first PR's should get the core functionality out in a way that is
straightforward, which will increase the odds that our design
decisions now can be easily extended to accommodate future
enhancements.
Besides that, this is a small PR, most of the lines being documentation.
Thanks. _I am a student in Chicago working independently on this project.
As of this week, I am working on this project full-time._
I am *always* reachable directly via instant message or video calling
in PyG's slack. My display name is @michael Fortunato
and my account link is https://torchgeometricco.slack.com/team/U08AMAGE318.
Please feel free to message or call me directly, at anytime, with
questions or comments. I am also always happy to meet if that helps get
things moving.
Thanks so much. Once again, for background on the entire plan please see my
comments on
the [original issue](#283)
along with the attached PDF of the plan. To get started with this PR,
I recommend viewing the rendered class documentation for
`torch.nn.conv.MeshCNNConv`. If there is a way for me to self host this
documentation, I will post an URL later today to it so you do not have
to clone my branch and render it yourself.
Best,
Michael
@michael Fortunato on
[Slack](https://torchgeometricco.slack.com/team/U08AMAGE318)1 parent 36aed7c commit f5650c0Copy full SHA for f5650c0
File tree
4 files changed
+1422
-0
lines changedFilter options
- docs/source/_figures
- test/nn/conv
- torch_geometric/nn/conv
4 files changed
+1422
-0
lines changed
0 commit comments