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

Add LocalFeatureStore for distributed training #7452

Merged
merged 9 commits into from
May 31, 2023

Conversation

ZhengHongming888
Copy link
Contributor

This code belongs to the part of the whole distributed training for PyG.

This class extends from FeatureStore and use the store dict to save the node/edge features and also use global_idx & id2index dict to save the global node ids and the id mapping between node ids and local feature index, these of which will be used by sampling and feature lookup stage.

We also include the unit test under /test/distributed folder to show how the node/edge feature will be saving/getting and verified by two kinds of method.

Any comments please let us know. thanks

@rusty1s rusty1s changed the title Add LocalFeatureStore for distributed training Add LocalFeatureStore for distributed training May 28, 2023
@ZhengHongming888
Copy link
Contributor Author

@rusty1s anything i need change here? :-) Thanks.

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #7452 (4fcba1d) into master (68fa7e7) will decrease coverage by 0.18%.
The diff coverage is 77.04%.

❗ Current head 4fcba1d differs from pull request most recent head 027ccc0. Consider uploading reports for the commit 027ccc0 to get more accurate results

@@            Coverage Diff             @@
##           master    #7452      +/-   ##
==========================================
- Coverage   91.61%   91.44%   -0.18%     
==========================================
  Files         445      446       +1     
  Lines       24717    24775      +58     
==========================================
+ Hits        22644    22655      +11     
- Misses       2073     2120      +47     
Impacted Files Coverage Δ
torch_geometric/distributed/local_feature_store.py 76.66% <76.66%> (ø)
torch_geometric/distributed/__init__.py 100.00% <100.00%> (ø)

... and 17 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@rusty1s rusty1s left a comment

Choose a reason for hiding this comment

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

@ZhengHongming888 I made some changes, PTAL. Especially, I introduced get_tensor_from_global_id such that global_id_to_index is solely a private attribute. I think this makes the logic a bit nicer.

Also left a TODO such that we could potentially compute the mapping much more memory-friendly, e.g., via utils.map_index functionality from PyG. PTAL.

@rusty1s rusty1s enabled auto-merge (squash) May 31, 2023 07:54
@rusty1s rusty1s merged commit 1e06255 into pyg-team:master May 31, 2023
@ZhengHongming888
Copy link
Contributor Author

Thanks @rusty1s yes it is very clear for me on these correction ! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants