Questions about pytorch lightning examples

[ashleve]

Hi, I have a couple of doubts about lightning example of GIN provided here: https://github.com/rusty1s/pytorch_geometric/blob/master/examples/pytorch_lightning/gin.py

1. Is this projection of batch to device really needed? I thought lightning takes care of such things automatically https://github.com/rusty1s/pytorch_geometric/blob/c786c02845df216ada316f6f535a72b565c9a424/examples/pytorch_lightning/gin.py#L96-L97
2. Shouldn't there be separate instance of accuracy metric for each of train, val and test? I was once told by lightning devs that using only one instance leads to incorrect value reduction over the epoch, especially in DDP mode (or maybe I misunderstood something) https://github.com/rusty1s/pytorch_geometric/blob/c786c02845df216ada316f6f535a72b565c9a424/examples/pytorch_lightning/gin.py#L87-L89
3. Why is inplace parameter of relu set to true? Does it make any difference in Sequential container? https://github.com/rusty1s/pytorch_geometric/blob/c786c02845df216ada316f6f535a72b565c9a424/examples/pytorch_lightning/gin.py#L68-L75
4. I'm pretty sure sinc_dist=True shouldn't be set when using built in metric api like Accuracy as explained in lightning docs here https://github.com/rusty1s/pytorch_geometric/blob/c786c02845df216ada316f6f535a72b565c9a424/examples/pytorch_lightning/gin.py#L109-L110 @tchaton

[rusty1s]

Thanks for starting this discussion:

1. It does not seem to be necessary anymore, but it was in a previous PL release which could not automatically convert custom objects to CUDA. I will remove the device transfer calls.
2. It is indeed recommended in the PL doc. However, I made some tests and it seems to work just fine with a single Accuracy() object. Hopefully, @tchaton can clarify.
3. It just saves a little bit of memory performing the ReLU in-place.
4. You are right that built-in metrics already support this :)

[rusty1s]

I updated the scripts according to your suggestions.