Add dist_context/RPC for distributed training
#7671
Conversation
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #7671 +/- ##
==========================================
- Coverage 91.95% 91.57% -0.39%
==========================================
Files 453 455 +2
Lines 25556 25657 +101
==========================================
- Hits 23500 23495 -5
- Misses 2056 2162 +106
... and 22 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
rusty1s
changed the title
dist_context/RPC for distributed training
There was a problem hiding this comment.
Thanks! Left some initial comments. The biggest one has to do with the use of global state here, where we may have a cleaner implementation if we remove some of these state and consolidate under classes that can be set as instance variables on the feature and graph store.
torch_geometric/distributed/rpc.py
Outdated
| def _rpc_remote_async_call(call_id, *args, **kwargs): | ||
| r""" Entry for rpc requests """ | ||
| return _rpc_call_pool.get(call_id).rpc_async_call(*args, **kwargs) | ||
|
|
||
|
|
||
| def _rpc_remote_sync_all(call_id, *args, **kwargs): | ||
| r""" Entry for rpc requests """ | ||
| return _rpc_call_pool.get(call_id).rpc_sync_call(*args, **kwargs) |
There was a problem hiding this comment.
Why do we need a global pool here? Can't we just pas the sync or async function and its args directly to rpc_request_async and rpc_request_sync below?
There was a problem hiding this comment.
For there we will use one pool it to distinguish the different call functions in mp processing for easy code understanding.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This code belongs to the part of the whole distributed training for PyG.
This class will do
These basic rpc functionality will be used in later feature lookup function after node sampling.
Any comments please let us know. thanks.