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

Fixing jit.trace tracing a constant number of nodes with add_self_loops #7330

Merged
merged 8 commits into from
May 10, 2023

Conversation

Vuenc
Copy link
Contributor

@Vuenc Vuenc commented May 9, 2023

Fix of issue #7226.

The problem I described came from the maybe_num_nodes function which computed int(edge_index.max()) + 1. The int() call made the computed number of nodes a constant for the torch.jit.trace function.

I fixed it with an if branch that is only executed while tracing. (I could not get the same code for all cases to work: the workaround used only for tracing now would also work for normal operation, but then would break when using torch.jit.script).

@Vuenc Vuenc requested a review from wsad1 as a code owner May 9, 2023 15:52
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Merging #7330 (e1cbbf8) into master (23c2836) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

@@           Coverage Diff           @@
##           master    #7330   +/-   ##
=======================================
  Coverage   91.53%   91.53%           
=======================================
  Files         438      438           
  Lines       24361    24364    +3     
=======================================
+ Hits        22298    22301    +3     
  Misses       2063     2063           
Impacted Files Coverage Δ
torch_geometric/utils/num_nodes.py 97.05% <100.00%> (+0.28%) ⬆️

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

@rusty1s rusty1s linked an issue May 9, 2023 that may be closed by this pull request
@rusty1s rusty1s changed the title Fixing jit.trace tracing a constant number of nodes with add_self_loops (fixes #7226) Fixing jit.trace tracing a constant number of nodes with add_self_loops May 9, 2023
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.

Thank you! I somehow missed the issue, so thanks for fixing it :) Cool solution.

@rusty1s rusty1s enabled auto-merge (squash) May 10, 2023 14:42
@rusty1s rusty1s merged commit 4d4c91a into pyg-team:master May 10, 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.

add_self_loops() tracing constant with torch.jit.trace()
2 participants