Skip to content

Add AeroGNNConv to torch_geometric.nn.conv#10285

Closed
bokveizen wants to merge 0 commit intopyg-team:masterfrom
bokveizen:master
Closed

Add AeroGNNConv to torch_geometric.nn.conv#10285
bokveizen wants to merge 0 commit intopyg-team:masterfrom
bokveizen:master

Conversation

@bokveizen
Copy link
Copy Markdown

  • Add new operator AeroGNNConv in nn.conv

@bokveizen bokveizen requested a review from EdisonLeeeee as a code owner May 18, 2025 20:56
@bokveizen
Copy link
Copy Markdown
Author

Initial PR. Still testing it.

@puririshi98
Copy link
Copy Markdown
Contributor

hi im intrigued by this. @bokveizen can you explain what the theoretical enefit is for using this model over a basic basline like GCN?
to get this merged can you add this as a model under torch_geometric.nn.models and then add as an option for examples/ogbn_train.py
then please run it and share the accuracy of this model vs GCN and GAT. If you show an experimental benefit or boost that would be great.
if you do show benefit it would be great to add this as an option to examples/ogbn_train_cugraph and some other examples that ill bring up for you as a secondary PR

@codecov
Copy link
Copy Markdown

codecov bot commented May 20, 2025

Codecov Report

Attention: Patch coverage is 18.91892% with 90 lines in your changes missing coverage. Please review.

Project coverage is 85.22%. Comparing base (c211214) to head (8019272).
Report is 55 commits behind head on master.

Files with missing lines Patch % Lines
torch_geometric/nn/conv/aero_gnn_conv.py 18.18% 90 Missing ⚠️

❌ Your patch check has failed because the patch coverage (18.91%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10285      +/-   ##
==========================================
- Coverage   86.11%   85.22%   -0.90%     
==========================================
  Files         496      497       +1     
  Lines       33655    34118     +463     
==========================================
+ Hits        28981    29076      +95     
- Misses       4674     5042     +368     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bokveizen
Copy link
Copy Markdown
Author

bokveizen commented May 20, 2025

hi im intrigued by this. @bokveizen can you explain what the theoretical enefit is for using this model over a basic basline like GCN?
to get this merged can you add this as a model under torch_geometric.nn.models and then add as an option for examples/ogbn_train.py
then please run it and share the accuracy of this model vs GCN and GAT. If you show an experimental benefit or boost that would be great.
if you do show benefit it would be great to add this as an option to examples/ogbn_train_cugraph and some other examples that ill bring up for you as a secondary PR

Hi! Thanks for the reply.
In short, this work was proposed to address some limitations of deep GNNs so that stacking more Conv layers can remain helpful.
See https://arxiv.org/abs/2306.02376 for more details.
Thanks for the suggestions. I will check them when I have some time 😃

@puririshi98
Copy link
Copy Markdown
Contributor

please also add a unit test under test/nn/models/test_aero_gnn.py

@puririshi98
Copy link
Copy Markdown
Contributor

puririshi98 commented May 20, 2025

In short, this work was proposed to address some limitations of deep GNNs so that stacking more Conv layers can remain helpful.
See https://arxiv.org/abs/2306.02376 for more details.

thanks, sounds good looking forward to the above mentioned changes

@bokveizen
Copy link
Copy Markdown
Author

bokveizen commented May 20, 2025

Thanks! One quick question: Could you clarify when you mentioned adding a model, did you mean adding a model instead of adding a conv layer or in addition to adding a conv layer?

  • IMO, you meant "instead of"

@puririshi98
Copy link
Copy Markdown
Contributor

please also update the changelog

@puririshi98
Copy link
Copy Markdown
Contributor

Thanks! One quick question: Could you clarify when you mentioned adding a model, did you mean adding a model instead of adding a conv layer or in addition to adding a conv layer?

  • IMO, you meant "instead of"

add it in addition to what you have now, so it can be used more easily, and use that easy model for the example

@puririshi98
Copy link
Copy Markdown
Contributor

please fix conflicts too

@bokveizen
Copy link
Copy Markdown
Author

Just want to say that I am busy with other things these days, and the PR is (and still will be) delayed for a while.

@puririshi98
Copy link
Copy Markdown
Contributor

Just want to say that I am busy with other things these days, and the PR is (and still will be) delayed for a while.

no prob

@puririshi98
Copy link
Copy Markdown
Contributor

ping em on slack at rishi puri if you want to have me review

@bokveizen
Copy link
Copy Markdown
Author

Will re-work on this from the latest version!

@bokveizen
Copy link
Copy Markdown
Author

Continued at #10573

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