torch geometric support + example notebook #863
Conversation
Adding support for torch geometric has so far been straight-forward since its data types also implement the `.to()` interface and use tensors in the background, they can basically be passed forward to the module which will deal with the data type accordingly. There are probably other places that might still fail, I can imagine that different data loaders (there are quite a [few][1]) might be problematic in terms of splitting X/y in batches, but this is a first step forward and we can address these issues once they arise. [1]: https://pytorch-geometric.readthedocs.io/en/latest/modules/loader.html
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Thanks a lot for adding this notebook. I'm happy to see that skorch can be made to work with PyTorch Geometric without too many adjustments. Given that those adjustments are not trivial, however, it's good to have a notebook to showcase them. Kudos for figuring out the minimal set of steps to take.
Could you please add a link to the notebooks README?
Regarding the notebook, I only have minor comments:
How to do batching with skorch + torch geometric will not be handled here.
I assume because it's not trivial? If so, could you please indicate that.
we need our own loader that strips the first dimension to match the predicted y and the labelled y in length
Alternatively, we could probably override get_loss and extract the first dimension from y in there, right? Maybe that is not better, but you mentioned that PyTorch Geometric provides a bunch of custom data loaders, so they could not be used in conjunction with RawLoader.
Regarding the end of the notebook, it would help to have some kind of conclusion about the result. E.g. why does the model not reach the same test score as in the linked tutorial?
Adding support for torch geometric has so far been straight-forward
since its data types also implement the
.to()interface and usetensors in the background, they can basically be passed forward to
the module which will deal with the data type accordingly.
There are probably other places that might still fail, I can imagine
that different data loaders (there are quite a few) might be
problematic in terms of splitting X/y in batches, but this is a
first step forward and we can address these issues once they arise.