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

Explainability Infection Benchmark #6222

Merged
merged 7 commits into from
Dec 16, 2022
Merged

Explainability Infection Benchmark #6222

merged 7 commits into from
Dec 16, 2022

Conversation

zechengz
Copy link
Member

@zechengz zechengz commented Dec 13, 2022

This PR implements Infection benchmark dataset as mentioned in #5817

Checklist

  • CHANGELOG
  • Documentation

@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #6222 (1f3286b) into master (bc55ff1) will decrease coverage by 1.86%.
The diff coverage is n/a.

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

@@            Coverage Diff             @@
##           master    #6222      +/-   ##
==========================================
- Coverage   86.38%   84.51%   -1.87%     
==========================================
  Files         376      372       -4     
  Lines       20942    20805     -137     
==========================================
- Hits        18090    17584     -506     
- Misses       2852     3221     +369     
Impacted Files Coverage Δ
torch_geometric/nn/models/dimenet_utils.py 0.00% <0.00%> (-75.52%) ⬇️
torch_geometric/nn/models/dimenet.py 14.90% <0.00%> (-52.76%) ⬇️
torch_geometric/profile/profile.py 33.33% <0.00%> (-25.39%) ⬇️
torch_geometric/nn/conv/utils/typing.py 83.75% <0.00%> (-15.00%) ⬇️
torch_geometric/nn/pool/asap.py 92.10% <0.00%> (-7.90%) ⬇️
torch_geometric/nn/inits.py 67.85% <0.00%> (-7.15%) ⬇️
torch_geometric/resolver.py 82.75% <0.00%> (-6.90%) ⬇️
torch_geometric/nn/dense/linear.py 85.40% <0.00%> (-5.91%) ⬇️
torch_geometric/transforms/add_self_loops.py 94.44% <0.00%> (-5.56%) ⬇️
torch_geometric/nn/models/attentive_fp.py 95.83% <0.00%> (-4.17%) ⬇️
... and 41 more

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

@github-actions github-actions bot removed the explain label Dec 13, 2022
Copy link
Contributor

@RexYing RexYing left a comment

Choose a reason for hiding this comment

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

Thanks! Overall it looks good to me. Just a few clarification questions.

test/datasets/test_infection_dataset.py Outdated Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
Copy link
Contributor

@RBendias RBendias 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 so much for adding this. It looks good to me. In general, I noticed that the description of the arguments (seeds, max_path_lenght and num_infected) do not include the information that if a list is provided, that this list needs to be as long as the number of graphs and that each element is used per graph.

torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
"""
def __init__(
self,
graph_generator: Union[GraphGenerator, str],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using a default here as well?

torch_geometric/datasets/infection_dataset.py Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
torch_geometric/datasets/infection_dataset.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the utils label Dec 16, 2022
@rusty1s rusty1s enabled auto-merge (squash) December 16, 2022 08:25
@rusty1s rusty1s merged commit d2f2503 into master Dec 16, 2022
@rusty1s rusty1s deleted the zecheng_infection branch December 16, 2022 08:29
@zechengz
Copy link
Member Author

Thanks @rusty1s @RBendias @RexYing !

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

4 participants