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

[GraphBolt] Add DiskBasedFeature example for DGL model #7624

Merged
merged 16 commits into from
Aug 6, 2024
Merged

[GraphBolt] Add DiskBasedFeature example for DGL model #7624

merged 16 commits into from
Aug 6, 2024

Conversation

Liu-rj
Copy link
Contributor

@Liu-rj Liu-rj commented Jul 31, 2024

Description

An example of the DiskBasedFeature for DGL model is added.

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
  • I've leverage the tools to beautify the python and c++ code.
  • The PR is complete and small, read the Google eng practice (CL equals to PR) to understand more about small PR. In DGL, we consider PRs with less than 200 lines of core code change are small (example, test and documentation could be exempted).
  • 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

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jul 31, 2024

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

  • @dgl-bot

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jul 31, 2024

Commit ID: 0685cc0

Build ID: 1

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

mfbalin
mfbalin requested changes Jul 31, 2024
View reviewed changes
Copy link
Collaborator

@mfbalin mfbalin left a comment

Choose a reason for hiding this comment

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

@frozenbugs Can you take a look as well?

examples/graphbolt/disk_based_feature/README.md Outdated Show resolved Hide resolved
examples/graphbolt/disk_based_feature/README.md Outdated Show resolved Hide resolved
examples/graphbolt/disk_based_feature/load_dataset.py Outdated Show resolved Hide resolved
examples/graphbolt/disk_based_feature/node_classification.py Outdated Show resolved Hide resolved
examples/graphbolt/disk_based_feature/node_classification.py Outdated Show resolved Hide resolved
examples/graphbolt/disk_based_feature/node_classification.py Outdated Show resolved Hide resolved
@mfbalin mfbalin requested a review from frozenbugs July 31, 2024 14:00
@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 1, 2024

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

  • @dgl-bot

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 1, 2024

Commit ID: 9569e84

Build ID: 2

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 1, 2024

Commit ID: b022723c50edd1f523d5d9a343fb6b8fe72288e0

Build ID: 3

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@Liu-rj
Copy link
Contributor Author

Liu-rj commented Aug 2, 2024

All the results are obtained on the save-aligned papers100M datasets now.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 2, 2024

Commit ID: 0c1812c7f8523295df604d4ddeabb6cf8576f8c9

Build ID: 4

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@mfbalin
Copy link
Collaborator

mfbalin commented Aug 2, 2024 •
edited
Loading

All the results are obtained on the save-aligned papers100M datasets now.

Did you do it on your local or is it that way for all users now?

@Liu-rj
Copy link
Contributor Author

Liu-rj commented Aug 2, 2024 •
edited
Loading

All the results are obtained on the save-aligned papers100M datasets now.

Did you do it on your local or is it that way for all users now?

Currently do it locally, will upload it to S3 later.

@Liu-rj
Copy link
Contributor Author

Liu-rj commented Aug 2, 2024 •
edited
Loading

All the results are obtained on the save-aligned papers100M datasets now.

Did you do it on your local or is it that way for all users now?

Currently do it locally, will upload it to S3 later.

The new papers100M dataset is uploaded to S3 now. Then users will download the save-aligned version.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 2, 2024

Commit ID: d177173

Build ID: 5

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 2, 2024

Commit ID: 20e24df

Build ID: 6

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

mfbalin
mfbalin approved these changes Aug 3, 2024
View reviewed changes
Copy link
Collaborator

@mfbalin mfbalin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Let's highlight the functionalities used in this example compared to examples/graphbolt/node_classification.py, update the runtime results with upto-date master branch and we can merge this PR.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 3, 2024

Commit ID: 13065d8

Build ID: 7

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 5, 2024

Commit ID: fe9679e

Build ID: 8

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@Liu-rj
Copy link
Contributor Author

Liu-rj commented Aug 5, 2024

@mfbalin I've added the comments and updated results. And I also add the comparison with in-mem performance. The results seem all reasonable now.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 5, 2024

Commit ID: ddf5c70c70ee2a5f319a2f100b43963a1ef58433

Build ID: 9

Status: ⚪️ CI test cancelled due to overrun.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 5, 2024

Commit ID: 2d12f8ffc97c3def8142a6abd5eb55920bed664a

Build ID: 10

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 5, 2024

Commit ID: 51dbfd233431d93a4fac5fa780a001db2aed0dfe

Build ID: 11

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 5, 2024

Commit ID: 7d9cb00d55e832cb247c90b13f474933d24c5542

Build ID: 12

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 6, 2024

Commit ID: 3d0b2955fc5220957a2c18d6b0da64727ed1a4d0

Build ID: 13

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@mfbalin mfbalin merged commit 90e7edd into dmlc:master Aug 6, 2024
2 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfbalin mfbalin mfbalin approved these changes

@frozenbugs frozenbugs Awaiting requested review from frozenbugs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Liu-rj @dgl-bot @mfbalin