dmlc
[Example][Refactor] Refactor RGAT example
Description
To resolve https://github.com/dmlc/dgl/issues/4411, this PR refactors rgat example according to the golden example https://github.com/dmlc/dgl/pull/4186.
Checklist
Please feel free to remove inapplicable items for your PR.
- [x] The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
- [x] Changes are complete (i.e. I finished coding on this PR)
- [x] All changes have test coverage
- [x] Code is well-documented
- [x] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
- [x] Related issue is referred in this PR
- [ ] If the PR is for a new model/paper, I've updated the example index here.
Changes
Similar code style as previous refactors
Results
| Dataset | Test acc. (before) | Mean | Test acc. (after) | Mean |
|---|---|---|---|---|
| ogbn-mag | [0.3492 0.3667 0.3543 0.3631 0.3648] | 0.3596 | [0.3588 0.3457 0.3617 0.3647 0.3852] | 0.3632 |
To trigger regression tests:
-
@dgl-bot run [instance-type] [which tests] [compare-with-branch]; For example:@dgl-bot run g4dn.4xlarge all dmlc/masteror@dgl-bot run c5.9xlarge kernel,api dmlc/master
@mufeili As I renamed file, the diff is not there... Sorry for any inconvenience. Pls feel free to let me change it back to ease the review process.
Commit ID: 8ee059004d8c2780a1aa41a607e1bff9aa90d967
Build ID: 1
Status: ✅ CI test succeeded
Report path: link
Full logs path: link
Commit ID: 02f07dc8d880ba6dce57e8c7d0bea5c66c89e251
Build ID: 2
Status: ✅ CI test succeeded
Report path: link
Full logs path: link
@mufeili As I renamed file, the diff is not there... Sorry for any inconvenience. Pls feel free to let me change it back to ease the review process.
I think you can get the benefit of the both by directly renaming the original file rather than create a new file and delete the original file.
I think you can get the benefit of the both by directly renaming the original file rather than create a new file and delete the original file.
Really? I don't know where I did wrong. I tried git mv rgat.py train.py, but it still shows the same file diff in github.
As for alternative, I reformed the commit tree of this PR. Please checkout this commit for review https://github.com/dmlc/dgl/pull/4530/commits/025cb1f1a4043cd4d40d4a6fa7d198077c42d1cd. The other commit is just for file renaming.
Commit ID: None
Build ID: 3
Status: ❌ CI test failed in Stage [Declarative: Checkout SCM].
Report path: link
Full logs path: link
Commit ID: None
Build ID: 4
Status: ❌ CI test failed in Stage [Declarative: Checkout SCM].
Report path: link
Full logs path: link
Commit ID: None
Build ID: 5
Status: ❌ CI test failed in Stage [Declarative: Checkout SCM].
Report path: link
Full logs path: link
Commit ID: 6d4060e332d5572b7fbedac622867c703fd43d73
Build ID: 6
Status: ❌ CI test failed in Stage [C++ CPU].
Report path: link
Full logs path: link
I think you can get the benefit of the both by directly renaming the original file rather than create a new file and delete the original file.
Really? I don't know where I did wrong. I tried
git mv rgat.py train.py, but it still shows the same file diff in github.As for alternative, I reformed the commit tree of this PR. Please checkout this commit for review 025cb1f. The other commit is just for file renaming.
Did this happen before? I think you also did something like that before. What if you rename the file via an IDE?
I am not sure... I tried in IDE(vscode) but it still doesn't work. I changed the file name back for now.
Commit ID: 5f61165d57bc060ae9efde85a021bd709dfce0ce
Build ID: 8
Status: ❌ CI test failed in Stage [Lint Check].
Report path: link
Full logs path: link
Commit ID: 93c4a79ded68a86b3b99ba7356ea834b68996ba1
Build ID: 9
Status: ❌ CI test failed in Stage [Lint Check].
Report path: link
Full logs path: link
Commit ID: 4f8a3464235e2af3b519521926fa9a5a244cd927
Build ID: 7
Status: ❌ CI test failed in Stage [C++ CPU].
Report path: link
Full logs path: link
Commit ID: 26f211f9d9088c7eee3514ebc0fd6591dc2fa3f1
Build ID: 10
Status: ✅ CI test succeeded
Report path: link
Full logs path: link
Commit ID: 56c684d9a724583b5c5bbf0c50bd21d0e383d9ac
Build ID: 11
Status: ✅ CI test succeeded
Report path: link
Full logs path: link
Commit ID: a2d8e533d444997b0cff5c2d63a8539697c60a1e
Build ID: 13
Status: ✅ CI test succeeded
Report path: link
Full logs path: link
@mufeili
I think the reason Git resists to show rename diff, but file create/deletion, is due to its rename detection mechanism. When the file diff > 50% (default), the git tool will show it as file delete/create, instead of rename. It is changeable though, for example, using git diff -M20%, or git diff --find-renames 20% means consider a delete/add pair to be a rename if more than 20% of the file has NOT changed.
Here are some references:
https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---find-renamesltngt
https://github.com/git/git/blob/142430338477d9d1bb25be66267225fb58498d92/Documentation/gitdiffcore.txt#L123-L129
https://github.com/git/git/blob/36f8e7ed7d72d2ac73743c3c2226cceb29b32156/Documentation/config/diff.txt#L126-L133
Sorry for the late review. It looks great in general and I've left some minor comments.