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

[Example][Refactor] Regolden graphsage example for future guide #4186

Merged
merged 12 commits into from
Jun 30, 2022

Conversation

chang-l
Copy link
Collaborator

@chang-l chang-l commented Jun 28, 2022

Description

Simplify from #4159
Focus on single-GPU, mini-batch node classification task (graphsage/node_classification.py) to ease the iterating process.

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented
  • To the best of my knowledge, examples are either not affected by this change,
    or have been fixed to be compatible with this change
  • Related issue is referred in this PR
  • If the PR is for a new model/paper, I've updated the example index here.

Changes

  • Arguments/keyword arguments naming consistent with doc
  • Data logging and output clean-up
  • Remove all cmd arguments, only keeping --gpu for simplicity (proper argument naming + doc should be clear enough)
  • Add comments for clarity
  • Bugfix: fanout correction from [15, 10, 5] --> [5, 10, 15]
  • README update

Tests

  • python3 node_classification.py --gpu 0
----Data statistics------'
    #Edges 123718280
    #Classes 47
    #Train samples 196615
    #Val samples 39323
    #Test samples 2213091
Epoch 00000 | Loss 1.3636 | Accuracy 0.8556 | GPU Mem(MB) 871.24
...
Epoch 00009 | Loss 0.3997 | Accuracy 0.8968 | GPU Mem(MB) 871.50

Test Accuracy 0.7658

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 28, 2022

To trigger regression tests:

  • @dgl-bot run [instance-type] [which tests] [compare-with-branch];
    For example: @dgl-bot run g4dn.4xlarge all dmlc/master or @dgl-bot run c5.9xlarge kernel,api dmlc/master

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 28, 2022

Commit ID: e6ebe29

Build ID: 1

Status: ❌ CI test failed in Stage [Torch CPU (Win64) Unit test].

Report path: link

Full logs path: link

@jermainewang jermainewang added the Release Candidate Candidate PRs for the upcoming release label Jun 29, 2022
@mufeili
Copy link
Member

mufeili commented Jun 29, 2022

Will examples/pytorch/graphsage_base break some links to the old GraphSAGE example? @jermainewang

@jermainewang
Copy link
Member

@chang-l I made some changes to the golden example. Please take a look.
@mufeili I believe Chang is creating a standalone file just for code review. I agree that we should merge it back to the original graphsage folder.

@mufeili
Copy link
Member

mufeili commented Jun 29, 2022

Done a first pass, some minor comments

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 29, 2022

Commit ID: 02799f81a71fc6af385df1dbad695658fda9ca48

Build ID: 2

Status: ✅ CI test succeeded

Report path: link

Full logs path: link

@jermainewang
Copy link
Member

@mufeili Addressed your comments expect the ones about inference. I feel that part may be changed quite significantly in the future so we may not want to focus on it too much. Plus, many full graph examples do not need it.

@chang-l
Copy link
Collaborator Author

chang-l commented Jun 30, 2022

@mufeili @jermainewang
Sure, I merged the two folders.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 287ce5db3c94abc81158ee470f526523deaabf36

Build ID: 6

Status: ✅ CI test succeeded

Report path: link

Full logs path: link

@mufeili
Copy link
Member

mufeili commented Jun 30, 2022

LGTM.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: de04ba6602d009d1380378f7dfa17a4dfc7f53f4

Build ID: 7

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 2ce258a

Build ID: 9

Status: ❌ CI test failed in Stage [C++ CPU].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 3971938d87f9d36d804b381cfaf1e73d5b0b498f

Build ID: 8

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 2ce258a

Build ID: 10

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: edacd7d

Build ID: 11

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: edacd7d

Build ID: 12

Status: ❌ CI test failed in Stage [C++ CPU (Win64)].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: edacd7d

Build ID: 13

Status: ❌ CI test failed in Stage [Torch CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 5d1da13

Build ID: 14

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jun 30, 2022

Commit ID: 94d0cc6

Build ID: 15

Status: ❌ CI test failed in Stage [Tensorflow CPU Unit test].

Report path: link

Full logs path: link

@jermainewang
Copy link
Member

I'll just merge it. Don't want to wait for CI, which is currently quite overloaded.

@jermainewang jermainewang merged commit b76d0ed into dmlc:master Jun 30, 2022
@chang-l chang-l deleted the example_refactor_graphsage branch June 30, 2022 17:01
@jermainewang jermainewang mentioned this pull request Aug 10, 2022
10 tasks
@frozenbugs frozenbugs removed the Release Candidate Candidate PRs for the upcoming release label Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants