-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add a normalize
parameter to dense_diff_pool
#4847
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4847 +/- ##
=======================================
Coverage 82.71% 82.71%
=======================================
Files 326 326
Lines 17520 17521 +1
=======================================
+ Hits 14491 14492 +1
Misses 3029 3029
Continue to review full report at Codecov.
|
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.
looks good to me.
Co-authored-by: Jinu Sunil <jinu.sunil@gmail.com>
dense_diffpool
dense_diffpool
normalize
parameter to dense_diff_pool
I did this modification because:
While I was using this function, I found the link_loss being exceptionally small and thus almost not contributing anything, making the graph structure hard to preserve and all nodes resulting in a few clusters. This is because adj.numel() are generally very large. After I recovered the result by multiplying adj.numel() back, results became reasonable.
According to the original paper, the link_loss is not divided by the size of adjacency matrix
image
After communicating with people on slack, I think it makes more sense for us to allow users to have an option of not being normalized.