GATConv ignores "edge_attr" without error message

[FrancisOWO]

🛠 Proposed Refactor

Description

The current version of GATConv and GATv2Conv handle the edge_attr parameter inconsistently in the edge_update function. When users forget to set edge_dim in the constructor but pass edge_attr in the forward function, GATv2Conv will raise an error, whereas GATConv will ignore edge_attr without informing the users, leading them to mistakenly believe that edge_attr has been used in the convolution.

I think it would be better to raise an error in this situation, and it is important to maintain consistency between GATConv and GATv2Conv in the handling of edge_attr.

For more details, please refer to Issue #810.

Reference

Originally posted by @FrancisOWO in #810 (comment)

Hi, I noticed that in the latest version (2.7.0) of the code, this assert statement has been removed from GATConv, but it remains in GATv2Conv, as shown in the figure below.

However, removing the assert statement leads to an issue: if a user passes edge_attr without setting edge_dim, the current GATConv will ignore edge_attr without informing the user, leading them to mistakenly believe that edge_attr has been used in the convolution.

I was not aware of this issue while using GATConv either, until I encountered an AssertionError when using GATv2Conv. Therefore, I think it would be better to retain the assert statement, or alternatively, to raise an error with some explanatory message, such as: require edge_dim to be set in the constructor when edge_attr is used.

I checked the commit history and found that this issue was caused by Commit 025b1cb (GATConv: require edge_dim to be set).

Do you think it would be better to retain the assert statement or raise an error?
At the very least, GATConv and GATv2Conv should be consistent in this part of the code.

Suggest a potential alternative/fix

I think it is important to maintain consistency between GATConv and GATv2Conv in the handling of edge_attr. It would be better to retain the assert statement, or alternatively, to raise an error with some explanatory message, such as: require edge_dim to be set in the constructor when edge_attr is used.