-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add the support of using WholeGraph distributed embedding to store/update sparse_emb #677
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
classicsong
reviewed
Dec 16, 2023
Refactor and simplify WholeGraph integration of Sparse Opt Bug fixes and improve tests
And slighty improve the code quality
@classicsong Here is an example of yaml file change, w.r.t this PR, to add wholegraph support for learnable embeddings: 44e4643 Note I only updated file gsgnn_lp.py for link pred tasks. |
classicsong
reviewed
Jan 18, 2024
To accommodate pytorch/DistDGL logic
classicsong
reviewed
Jan 25, 2024
classicsong
reviewed
Feb 1, 2024
classicsong
approved these changes
Feb 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update on 12/11/2023:
Most added code is covered by unit tests, except the sparse optimizer step function in graphstorm. To compensate it and gain more confident for the integrated feature, I also added an independent unit test
test_wg_sparse_opt.py
, which has no dependency on graphstorm, to show the integrated wholegraph sparse optimizer can generate close enough embs vs. torch sparse adam optimizer.Current code should be compatible now, ie., all unit tests should pass (except the fails due to #685). This PR is ready for review.
Updated todos:
Test/debug forsparse_optimizer.step()
to show the updates are consistent between WholeGraph vs. DistDGL vs. PyTorchJust like using WholeGraph distributed feature store for gathering, this PR tries to add the support of using WholeGraph to manage
sparse_embs
to unblock learnable embeddings with WholeGraph.For now, it is still a draft PR. The working test only covers save/load sparse embeddings using WholeGraph.
Todos:
sparse_optimizer.step()
to show the updates are consistent between WholeGraph vs. DistDGL vs. PyTorch@zheng-da @classicsong @isratnisa @nvcastet @TristonC