Add allow_zero_in_degree option for gat and gcn models

awslabs / dgl-lifesci

Python package for graph neural networks in chemistry and biology

Apache License 2.0

714 stars 147 forks source link

Add allow_zero_in_degree option for gat and gcn models #203

Closed astolman closed 1 year ago

astolman commented 1 year ago

This adds the allow_zero_in_degree option to be set in the model. This option exists in the base dgl layer objects. This change just adds the option to the GCN and GAT constructors to pass it down to the dgl layer.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

astolman commented 1 year ago

Question for the dgllife maintainers: if this PR gets accepted, what is the timeline for having a new version propagated to the conda package?

mufeili commented 1 year ago

We now only provide pip wheels. If we merge this PR, I can make a new release immediately if you want.

astolman commented 1 year ago

We now only provide pip wheels. If we merge this PR, I can make a new release immediately if you want.

Yeah, that would be great. Thank you.

astolman commented 1 year ago

How long does it normally take CI? All I've seen is yellow circle for a couple days.

mufeili commented 1 year ago

How long does it normally take CI? All I've seen is yellow circle for a couple days.

CI is currently down. I will test the code locally after reviewing and modifying it.

mufeili commented 1 year ago

Before we merge this PR:

When do you need to set allow_zero_in_degree to True?
You can add your name here if you want.

astolman commented 1 year ago

* When do you need to set `allow_zero_in_degree` to True?
Whenever the graph you feed into the model has some zero in degree node. This can easily happen on directed graphs or as a result of subgraph sampling techniques.

astolman commented 1 year ago

Added myself to the contributors. Thanks for the review!

mufeili commented 1 year ago

* When do you need to set `allow_zero_in_degree` to True?
Whenever the graph you feed into the model has some zero in degree node. This can easily happen on directed graphs or as a result of subgraph sampling techniques.

For these cases, I think it's more common to add self-loops or reverse edges. I'll merge the PR for now.

astolman commented 1 year ago

* When do you need to set `allow_zero_in_degree` to True?
Whenever the graph you feed into the model has some zero in degree node. This can easily happen on directed graphs or as a result of subgraph sampling techniques.
For these cases, I think it's more common to add self-loops or reverse edges. I'll merge the PR for now.

If you're just feeding a normal HeteroGraph to the models this works. But when the graph is created via create_block it doesn't seem to let you add self loops.

mufeili commented 1 year ago

* When do you need to set `allow_zero_in_degree` to True?
Whenever the graph you feed into the model has some zero in degree node. This can easily happen on directed graphs or as a result of subgraph sampling techniques.
For these cases, I think it's more common to add self-loops or reverse edges. I'll merge the PR for now.
If you're just feeding a normal HeteroGraph to the models this works. But when the graph is created via create_block it doesn't seem to let you add self loops.

There should be a workaround. I believe people have raised this point before. In that case, you might want to check the existing DGL issues or opening a new issue.