Contributing more pooling and convolution layers

[LeviBorodenko]

Hey,
I have implementations of the following few graphical neural network components:

Convolutions:

• Diffusion based graph convolution (as part of my GARNN implementation)
• Graph convolutions in the sense of M. Zhang et al., 2018 (as part of my DGCNN implementation)

Pooling:

• SortPooling (Also as part my DGCNN implementation)

I would love to add these to your framework however, I am a bit lost with all the requirements that my implementations need to fulfil in order to integrate seamlessly. Is there any guide for what tests, features or properties layers need to have?

Best,
Levi

[danielegrattarola]

Hi,

I'd be happy to add your code to the framework. I should probably improve the contribution guidelines, here's more or less what it would be good to have:

• Message-passing layers should go in their own files under layers.convolutional;
• Make sure that your layer has a preprocess() static method if there is any pre-processing to be done to the adjacency matrix;
• Make sure that all functions needed for Keras are implemented (your layer should extend GraphConv);
• SortPool should be added as a class in layers/pooling/globalpool.py;
• See the documentation in the other layers for how to format docstrings (it's important that the format is that same so that the docs can be built automatically);

Tests are a bit different. It's better if you copy-paste what I did for other methods, or just open a PR and I'll add the test cases myself.

I've never had big PRs so it's better if we work together on this one :D
Just open a PR and we'll go from there.

Cheers,
Daniele

[LeviBorodenko]

Okay, wonderful!
I will give myself a week to familiarise myself with your codebase and will open a PR soon thereafter :)

Best,
Levi

[LeviBorodenko]

Is there any place where I can put utility layers used for convolutional layers without them cluttering the convolutional layer docs?

[danielegrattarola]

Sure! If it's a proper Layer you can put it in layers/base.py, whereas for utility functions you can modify/create files in layers/ops/.

[LeviBorodenko]

Alright, I believe all my implementations have now been merged into the development branch and this issue can be closed.

Now that I feel comfortable with this framework, I'd really like to contribute more. Is there any to-do list I can consult to help with further development?

[danielegrattarola]

Thanks again for contributing! I will be merging the new layers into master soon and pushing them to PyPi as well.
I would say that any additional convolutional or pooling layers would be a good contribution. I have thought about creating a general abstraction for message-passing layers for a while now, I will take this chance to start working on it over the following days. This should make future contributions easier.

I am also starting a TODO list in Projects to keep track of what is missing in the library.

Cheers,
Daniele

[danielegrattarola]

FYI I have just pushed to develop a MessagePassing class that should make it easier to implement single-mode layers.

[LeviBorodenko]

Very nice!

Out of curiosity, is disjoint mode supposed to be an internal data storage format, mainly served by spektral's data loader etc or do we expect many users to have their own data in that format? Because apart from TU, I am not aware of many datasets having that format.

Furthermore, I expect that many users will have batch-mode signal and adjacency data with the node dimension being ragged. It may be worth providing methods that allow to work with graph datasets that have a ragged node dimension.
To that end, I think either looking into tf.RaggedTensor or providing a function that creates batches of graphs with same size might be useful.

Best,
Levi

[danielegrattarola]

Disjoint mode solves exactly the problem of having to work with ragged tensors. I have looked into tf.ragged several times since I started this project, but there's always some problem that makes it impossible to move the whole framework to a ragged representation of graphs.

This way of representing graph is not only limited to the TU data, but is a fairly common way of representing graphs (also sometimes called ad "edge list"... it's all the same, really). The new OGB datasets are represented like this, for instance, and also many network datasets found online.

I think it's pretty convenient, and I don't see a clear advantage of batch mode over disjoint, especially because it is impossible to store batch data as sparse matrices.
Other frameworks like Pytorch Geometric also use disjoint mode for basically all of their layers.

So let's say that you have two graphs with a different number of nodes, stored in lists of matrices:

A_list = [A_1, A_2]
X_list = [X_1, X_2]

You can convert them to batch mode (all graphs have the same size):

from spektral.utils.data import numpy_to_batch
A_batch, X_batch = numpy_to_batch(A_list, X_list)

Or you can convert them to disjoint mode:

from spektral.utils.data import numpy_to_disjoint
A_disjoint, X_disjoint, I_disjoint = numpy_to_disjoint(A_list, X_list)

Note that I_disjoint is only needed for pooling, it tells you which nodes belong to which sub-graph.

I hope that answers your doubts.

Cheers,
D