benedekrozemberczki / pytorch_geometric_temporal

PyTorch Geometric Temporal: Spatiotemporal Signal Processing with Neural Machine Learning Models (CIKM 2021)
MIT License
2.61k stars 367 forks source link

[Bug in EvolveGCNO/EvolveGCNH] the state of the weight is not maintained correctly #220

Closed rishiagarwal2000 closed 1 year ago

rishiagarwal2000 commented 1 year ago

Hi, I found a similar issue that was closed earlier. I did go over the discussion, however, I still think that the implementation is incorrect.

This is the original implementation. Here, self.weight is is not updated during the forward pass. So, if we do 2 consecutive forward passes, in both the forward passes, we start with the same value for self.weight. While we need the value of self.weight to be updated at the end of first forward pass, and use the updated value for the second forward pass.

image

Also, in the earlier issue the following explanation was given to justify the existing implementation. However, it is not a correct justification, as in the existing implementation we do not index W while assigning the new values, instead we simply assign W to the output of the recurrent layer. image

Following two code snippets are the proposed changes that work correctly.

Screenshot 2023-03-22 at 5 06 16 PM Screenshot 2023-03-22 at 5 05 26 PM