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.

Sign up for GitHub

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

Jump to bottom

Add NeighborLoader benchmark suite #4815

Merged
merged 7 commits into from
Jun 24, 2022
Merged

Add NeighborLoader benchmark suite #4815

merged 7 commits into from
Jun 24, 2022

Conversation

mszarma
Copy link
Contributor

@mszarma mszarma commented Jun 17, 2022 •
edited
Loading

  • Adding loader folder to benchmarks
  • Adding NeighborLoader benchmark for sampling on ognb-arxiv, ogbn-products and ogbn-mag

@codecov
Copy link

codecov bot commented Jun 17, 2022 •
edited
Loading

Codecov Report

Merging #4815 (7610c86) into master (654c10b) will not change coverage.
The diff coverage is n/a.

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

@@           Coverage Diff           @@
##           master    #4815   +/-   ##
=======================================
  Coverage   82.80%   82.80%           
=======================================
  Files         329      329           
  Lines       17713    17713           
=======================================
  Hits        14668    14668           
  Misses       3045     3045

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 654c10b...e72f6a4. Read the comment docs.

@rusty1s rusty1s changed the title [benchmark] Add NeighborLoader bench Add NeighborLoader benchmark suite Jun 19, 2022
rusty1s
rusty1s approved these changes Jun 20, 2022
View reviewed changes
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! Left a few minor comments.

benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated Show resolved Hide resolved
benchmark/sampling/neighborloader_bench.py Outdated
f'times={times} average_time={average_time}')
print('Validation sampling')
for batch_size in args.eval_batch_sizes:
val_loader = NeighborLoader(data, num_neighbors=[-1],
Copy link
Member

Choose a reason for hiding this comment

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

Add some comment why this benchmark is needed/why it relates to validation sampling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially i thought about checking also some scenario with sampling all neighbors like more inference specific sampling? i changed the code - let me know if that make sense or there are some better real scenarios? WDYT?

@rusty1s rusty1s merged commit 7a89399 into pyg-team:master Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rusty1s rusty1s rusty1s approved these changes

Assignees

@mszarma mszarma

Labels
0 - Priority P0 benchmark feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mszarma @rusty1s