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

Adding trim_to_layer utility function #6661

Merged
merged 15 commits into from
Mar 20, 2023

Conversation

mszarma
Copy link
Contributor

@mszarma mszarma commented Feb 10, 2023

Adding hierarchical graph adjacency matrix feature to significantly improve performance

Contributors: @rBenke @andreazanetti

Copy link
Member

@rusty1s rusty1s 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 this PR. I have a few questions:

  • Can we split this PR into multiple such that we first integrate sampled_info into the NeighborLoader before we think about integration in MessagePassing/benchmark scripts?
  • I am not entirely sure why we need the concept of a HierarchicalSparseTensor. Can't we just let NeighborLoader return a list of sliced edge indices/sparse tensors?
  • I would like to avoid adding any logic of this to MessagePassing and its instances. IMO, any customization of x and edge_index should happen outside of it. Would that be possible?

torch_geometric/loader/neighbor_loader.py Outdated Show resolved Hide resolved
torch_geometric/loader/node_loader.py Outdated Show resolved Hide resolved
torch_geometric/loader/node_loader.py Outdated Show resolved Hide resolved
torch_geometric/sampler/base.py Outdated Show resolved Hide resolved
@mszarma
Copy link
Contributor Author

mszarma commented Feb 27, 2023

Thanks for this PR. I have a few questions:

  • Can we split this PR into multiple such that we first integrate sampled_info into the NeighborLoader before we think about integration in MessagePassing/benchmark scripts?
  • I am not entirely sure why we need the concept of a HierarchicalSparseTensor. Can't we just let NeighborLoader return a list of sliced edge indices/sparse tensors?
  • I would like to avoid adding any logic of this to MessagePassing and its instances. IMO, any customization of x and edge_index should happen outside of it. Would that be possible?

Thanks @rusty1s for the review. Let's proceed first with pyg-lib part.

rusty1s added a commit to pyg-team/pyg-lib that referenced this pull request Feb 28, 2023
It's enabling the hierarchical tensor usage
and significant performance improvement

PyG part: pyg-team/pytorch_geometric#6661

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: rusty1s <matthias.fey@tu-dortmund.de>
to significantly improve performance
@mszarma mszarma changed the title Adding hierarchical graph adjacency matrix feature Adding trim_to_layer utils function Mar 3, 2023
@mszarma mszarma requested a review from rusty1s March 3, 2023 15:04
@mszarma mszarma marked this pull request as ready for review March 3, 2023 15:09
@rusty1s rusty1s changed the title Adding trim_to_layer utils function Adding trim_to_layer utility function Mar 4, 2023
@rusty1s rusty1s enabled auto-merge (squash) March 20, 2023 12:30
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #6661 (7b3b6fc) into master (bf96b63) will increase coverage by 0.00%.
The diff coverage is 92.68%.

❗ Current head 7b3b6fc differs from pull request most recent head 3c1dde7. Consider uploading reports for the commit 3c1dde7 to get more accurate results

@@           Coverage Diff           @@
##           master    #6661   +/-   ##
=======================================
  Coverage   91.46%   91.46%           
=======================================
  Files         431      432    +1     
  Lines       23458    23498   +40     
=======================================
+ Hits        21455    21492   +37     
- Misses       2003     2006    +3     
Impacted Files Coverage Δ
torch_geometric/sampler/base.py 98.02% <ø> (ø)
torch_geometric/nn/models/basic_gnn.py 89.18% <87.50%> (-0.08%) ⬇️
torch_geometric/utils/trim_to_layer.py 92.59% <92.59%> (ø)
torch_geometric/nn/summary.py 100.00% <100.00%> (ø)
torch_geometric/typing.py 100.00% <100.00%> (ø)
torch_geometric/utils/__init__.py 100.00% <100.00%> (ø)

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

@rusty1s rusty1s merged commit a6db0ab into pyg-team:master Mar 20, 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.

2 participants