snap-stanford / ogb

Benchmark datasets, data loaders, and evaluators for graph machine learning
https://ogb.stanford.edu
MIT License
1.89k stars 397 forks source link

Support e2e time and profile in examples/nodeproppred/products/gnn.py #401

Closed yanbing-j closed 1 year ago

yanbing-j commented 1 year ago

This PR is to add e2e time and profile support in examples/nodeproppred/products/gnn.py for both inference and training.

yanbing-j commented 1 year ago

Hi @rusty1s , could you please help review this PR?

rusty1s commented 1 year ago

Code-wise this looks all good, but I am not sure if we should merge any benchmarking to the OGB repo. Can't we integrate this into torch_geometric/benchmark?

yanbing-j commented 1 year ago

Can we move this gnn.py to torch_geometric/benchmark/training? Or a new directory under torch_geometric/benchmark?

rusty1s commented 1 year ago

I think we can directly integrate this into training_benchmark.py with a full-batch option. WDYT?

yanbing-j commented 1 year ago

I create a PR to move this gnn.py to torch_geometric/bench,ark/ogbn-benchmark. https://github.com/pyg-team/pytorch_geometric/pull/6169 Since it can run both inference and training benchmark, so I didn't put it in training_benchmark. And if other examples from ogb need to move into pyg, maybe creating a new ogbn-benchmark is a better one. WDYT?

rusty1s commented 1 year ago

I personally don't think we need special benchmarks for ogb datasets, we should just integrate them into existing ones. There is also support for ogbn-products in training_benchmark.py already (see here). Can you clarify the purpose?

yanbing-j commented 1 year ago

Understand. The full-batch ogbn-products perform better than mini-batch in training_benchmark, and spmm_reduce takes large portion while mini-batch doesn't. Never mind. For current stage, we can use full-batch from ogb to benchmark. And will integrate into training_benchmark as a full-batch option later.

rusty1s commented 1 year ago

Sounds good! Thanks.

yanbing-j commented 1 year ago

Currently, this PR is still needed as the example in the blog. Will close this when it is integrated into the existing benchmark in PyG.