Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug in utils.bipartite_subgraph #6654

Merged
merged 26 commits into from
Feb 13, 2023
Merged

Fixed bug in utils.bipartite_subgraph #6654

merged 26 commits into from
Feb 13, 2023

Conversation

toenshoff
Copy link
Contributor

@toenshoff toenshoff commented Feb 9, 2023

The PR fixes a bug in the utils.bipartite_subgraph method, which crashes if src_subset contains Boolean values but dst_subset contains integers.

Consider the following example:

import torch
from torch_geometric.utils import bipartite_subgraph

subset = (torch.tensor([True, True, False]), torch.tensor([0, 3]))
edge_index = torch.tensor([[0, 1, 2], [0, 0, 3]])
sub_edge_index = bipartite_subgraph(subset, edge_index)

With torch_geometric==2.3.0 we obtain the following error:

Traceback (most recent call last):
  File "/home/jan/git/gerd/pygtest.py", line 8, in <module>
    sub_edge_index = bipartite_subgraph(subset, edge_index)
  File "/home/jan/miniconda3/envs/gerd/lib/python3.9/site-packages/torch_geometric/utils/subgraph.py", line 178, in bipartite_subgraph
    edge_mask = src_subset[edge_index[0]] & dst_subset[edge_index[1]]
IndexError: index 3 is out of bounds for dimension 0 with size 2

The method determines whether or not a cast of the masks is needed by only checking the dtype of the source mask, which causes this issue.

While this seems like a rare edge case, this issue does cause the HeteroData.subgraph method to crash when subset_dict uses Boolean masks and does not contain some ntypes as keys, as integer tensors are then added for the missing ntypes.

Since #6218, HeteroData.subgraph does retain ntypes that are absent from subset_dict. I have also updated the doc string of the method to reflect this change, since it was still describing the previous behavior.

@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Merging #6654 (1d585e2) into master (6196ab3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 1d585e2 differs from pull request most recent head a1585e6. Consider uploading reports for the commit a1585e6 to get more accurate results

@@           Coverage Diff           @@
##           master    #6654   +/-   ##
=======================================
  Coverage   88.97%   88.97%           
=======================================
  Files         423      423           
  Lines       22957    22958    +1     
=======================================
+ Hits        20425    20427    +2     
+ Misses       2532     2531    -1     
Impacted Files Coverage Δ
torch_geometric/data/hetero_data.py 90.81% <ø> (ø)
torch_geometric/utils/subgraph.py 100.00% <100.00%> (ø)
torch_geometric/data/storage.py 82.68% <0.00%> (+0.24%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rusty1s rusty1s changed the title Fixed Bug in utils.bipartite_subgraph and Updated Documentation Fixed bug in utils.bipartite_subgraph Feb 10, 2023
Copy link
Member

@wsad1 wsad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing.

torch_geometric/utils/subgraph.py Show resolved Hide resolved
@wsad1 wsad1 enabled auto-merge (squash) February 13, 2023 12:25
@wsad1 wsad1 merged commit 2b293e7 into pyg-team:master Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants