[NN] Add EdgeGAT operator #5282

schmidt-ju · 2023-02-10T13:04:49Z

Description

Hey,

I added the EdgeGAT operator of the Robotics and Automation Letters (RA-L) paper SCENE: Reasoning about Traffic Scenes using Heterogeneous Graph Neural Networks.
The already available EGAT operator does only include edge feature for the calculation of the attention coefficients.

The operator of this PR, namely EdgeGAT, is more advanced: It also includes the edge features directly for the node update and not only for the calculation of the attention coefficients.

Checklist

Please feel free to remove inapplicable items for your PR.

The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
I've leverage the tools to beautify the python and c++ code.
All changes have test coverage
Code is well-documented
To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Looking forward to hearing from you!

Julian

dgl-bot · 2023-02-10T13:05:14Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-10T13:05:27Z

Commit ID: 3964ac09506365ca7d43e5ad3ab2e1f86c7e259c

Build ID: 1

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

schmidt-ju · 2023-02-10T15:37:29Z

Hey,

I am not sure, why the CI is failing. I just recreated the test cases and they seem to work on my local machine.

Also, I am not able to access the produced logs.
There is probably something that I am missing right now. Can you help me?

Julian

BarclayII · 2023-02-11T14:21:44Z

@dgl-bot

dgl-bot · 2023-02-11T14:23:56Z

Commit ID: 420d5b590c4b33fc05f34af9685dad0a4d16dad6

Build ID: 2

Status: ❌ CI test failed in Stage [Lint Check].

Report path: link

Full logs path: link

BarclayII · 2023-02-11T14:29:03Z

@schmidt-ju Hi Julian, currently CI requires authentication from one of the core developers so someone from us has to invoke the CI manually. And after the CI is invoked and an error other than "Authentication failed" is shown, you will have to click on the Report path link and find the logs like below:

Looks like there is lint issues. Could you take a look?

Please feel free to nudge us if you have any problems. Thanks!

dgl-bot · 2023-02-11T16:41:44Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-11T16:41:56Z

Commit ID: 633232fda921d588c6ed4f27150d292df9c471c4

Build ID: 3

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

schmidt-ju · 2023-02-11T16:44:33Z

Hey @BarclayII,

thanks for the information!
I fixed the lint issues (to the best of my knowledge^^). At least on my local machine the task_lint.sh script does not throw errors anymore.

dgl-bot · 2023-02-11T16:44:58Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-11T16:45:10Z

Commit ID: acfdfe0

Build ID: 4

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

BarclayII · 2023-02-13T04:26:39Z

Seems that there's still lint issues. You could try lintrunner -a to apply the lint fixes automatically.

dgl-bot · 2023-02-14T08:25:44Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-14T08:25:58Z

Commit ID: 66165d4

Build ID: 5

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

BarclayII · 2023-02-14T08:57:44Z

The files are python/dgl/nn/pytorch/conv/edgegatconv.py and python/dgl/nn/pytorch/conv/__init__.py.

dgl-bot · 2023-02-14T10:41:20Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-14T10:41:33Z

Commit ID: 1007b66195ad5d0b74570648a805223dd5a78bbf

Build ID: 6

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

schmidt-ju · 2023-02-14T10:47:50Z

Hey @BarclayII,

I fixed the lint issues in the two mentioned files using lintrunner.
According to the "Checks" tab, there are still some issues in tests/pytorch/test_nn.py.

My modifications in this file are limited to two new test methods. The linter, however, shows errors in the whole file and not limited to my small changes.
Is this intentional?
I can reformat the whole file, but I don't think that this is something you want.

Looking forward to hearing from you.

Julian

dgl-bot · 2023-02-14T10:48:50Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-14T10:49:02Z

Commit ID: b8ea3a3

Build ID: 7

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

frozenbugs · 2023-02-23T02:41:16Z

@dgl-bot

frozenbugs · 2023-02-23T02:41:45Z

@schmidt-ju Can you fix the conflict, I think the lint issue should be addressed.

dgl-bot · 2023-02-24T09:56:38Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-02-24T09:56:50Z

Commit ID: d5bcd8a

Build ID: 10

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

schmidt-ju · 2023-02-24T10:00:32Z

Hey @frozenbugs,
I resolved the conflicts. Lint issues are now gone, too (because of the changes in the test_nn.py file).

frozenbugs · 2023-02-27T02:40:58Z

@dgl-bot

dgl-bot · 2023-02-27T03:26:22Z

Commit ID: 13cf6e0487aa756b95bc61b0eda2237110ff9b77

Build ID: 11

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

python/dgl/nn/pytorch/conv/edgegatconv.py

frozenbugs · 2023-02-28T02:21:30Z

python/dgl/nn/pytorch/conv/edgegatconv.py

+from ...functional import edge_softmax
+
+# pylint: enable=W0235
+class EdgeGATConv(nn.Module):


Maybe mention SCENE in the name of EdgeGATConv? Since we already have another similar egatconv https://github.com/dmlc/dgl/blob/master/python/dgl/nn/pytorch/conv/egatconv.py

python/dgl/nn/pytorch/conv/edgegatconv.py

BarclayII

LGTM, we can merge it after the code style comments are addressed.

frozenbugs · 2023-02-28T02:31:29Z

Overall LGTM, thanks for using and contributing to DGL. @BarclayII will take another look then approve.

dgl-bot · 2023-03-03T10:24:29Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-03-03T10:24:42Z

Commit ID: c079d1b

Build ID: 12

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2023-03-03T10:46:43Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-03-03T10:46:56Z

Commit ID: 35d9ca1

Build ID: 13

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

schmidt-ju · 2023-03-03T10:53:46Z

Hey @BarclayII,

thanks for your overall positive feedback.

I addressed all of your comments.
The only thing left is @frozenbugs comment:

Maybe mention SCENE in the name of EdgeGATConv? Since we already have another similar egatconv https://github.com/dmlc/dgl/blob/master/python/dgl/nn/pytorch/conv/egatconv.py

I have talked to my co-author about this and we would like to leave the name as it is.
This makes the name in the paper and in DGL consistent with each other.

Looking forward to hearing from you!

Best

Julian

BarclayII · 2023-03-06T01:51:01Z

@dgl-bot

dgl-bot · 2023-03-06T02:41:30Z

Commit ID: ceee29ce898b6190f69567e41a633be29850b44f

Build ID: 14

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

schmidt-ju · 2023-04-08T17:49:20Z

Hey @BarclayII,

do you already have a rough estimate of when the merge into the main branch will be carried out?

Julian

frozenbugs · 2023-04-09T15:00:39Z

Hey @BarclayII,

do you already have a rough estimate of when the merge into the main branch will be carried out?

Julian

Hi Julian, sorry for the trouble, just merged the PR.
(I am new to github, and I thought you can merge also long as we approve)

Co-authored-by: Quan (Andy) Gan <coin2028@hotmail.com>

Add EdgeGAT operator

6c45d9e

Fix comments and linting

c66cd17

Merge branch 'master' into master

acfdfe0

Merge branch 'master' into master

66165d4

Fix linting with lintrunner -a

6d6a24b

frozenbugs requested review from frozenbugs and BarclayII February 23, 2023 02:41

schmidt-ju force-pushed the master branch from b8ea3a3 to 6d6a24b Compare February 24, 2023 09:45

schmidt-ju added 4 commits February 24, 2023 10:46

Rebase on upstream

d81b57a

Fix comments and linting

ad05ef1

Fix linting with lintrunner -a

817fd2f

Fix conflicts

d5bcd8a

frozenbugs reviewed Feb 28, 2023

View reviewed changes

BarclayII reviewed Feb 28, 2023

View reviewed changes

python/dgl/nn/pytorch/conv/edgegatconv.py Show resolved Hide resolved

python/dgl/nn/pytorch/conv/edgegatconv.py Show resolved Hide resolved

BarclayII approved these changes Feb 28, 2023

View reviewed changes

Merge branch 'dmlc:master' into master

c079d1b

Fix comment formatting

35d9ca1

frozenbugs approved these changes Mar 4, 2023

View reviewed changes

frozenbugs merged commit 97286f9 into dmlc:master Apr 9, 2023

DominikaJedynak pushed a commit to DominikaJedynak/dgl that referenced this pull request Mar 12, 2024

[NN] Add EdgeGAT operator (dmlc#5282)

7f3f444

Co-authored-by: Quan (Andy) Gan <coin2028@hotmail.com>

This pull request was closed.

[NN] Add EdgeGAT operator #5282

[NN] Add EdgeGAT operator #5282

Conversation

schmidt-ju commented Feb 10, 2023

Description

Checklist

dgl-bot commented Feb 10, 2023

dgl-bot commented Feb 10, 2023

schmidt-ju commented Feb 10, 2023

BarclayII commented Feb 11, 2023

dgl-bot commented Feb 11, 2023

BarclayII commented Feb 11, 2023

dgl-bot commented Feb 11, 2023

dgl-bot commented Feb 11, 2023

schmidt-ju commented Feb 11, 2023 • edited Loading

dgl-bot commented Feb 11, 2023

dgl-bot commented Feb 11, 2023

BarclayII commented Feb 13, 2023

dgl-bot commented Feb 14, 2023

dgl-bot commented Feb 14, 2023

BarclayII commented Feb 14, 2023

dgl-bot commented Feb 14, 2023

dgl-bot commented Feb 14, 2023

schmidt-ju commented Feb 14, 2023

dgl-bot commented Feb 14, 2023

dgl-bot commented Feb 14, 2023

frozenbugs commented Feb 23, 2023

frozenbugs commented Feb 23, 2023

dgl-bot commented Feb 24, 2023

dgl-bot commented Feb 24, 2023

schmidt-ju commented Feb 24, 2023

frozenbugs commented Feb 27, 2023

dgl-bot commented Feb 27, 2023

frozenbugs Feb 28, 2023

Choose a reason for hiding this comment

BarclayII left a comment

Choose a reason for hiding this comment

frozenbugs commented Feb 28, 2023

dgl-bot commented Mar 3, 2023

dgl-bot commented Mar 3, 2023

dgl-bot commented Mar 3, 2023

dgl-bot commented Mar 3, 2023

schmidt-ju commented Mar 3, 2023

BarclayII commented Mar 6, 2023

dgl-bot commented Mar 6, 2023

schmidt-ju commented Apr 8, 2023

frozenbugs commented Apr 9, 2023

schmidt-ju commented Feb 11, 2023 •

edited

Loading