Skip to content

Concurrently Issue All dist_feature.async_get()Requests in Base Sampler _collate_fn()#690

Open
jchmura-sc wants to merge 5 commits into
mainfrom
jchmura/concurrent_async_get
Open

Concurrently Issue All dist_feature.async_get()Requests in Base Sampler _collate_fn()#690
jchmura-sc wants to merge 5 commits into
mainfrom
jchmura/concurrent_async_get

Conversation

@jchmura-sc

@jchmura-sc jchmura-sc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Close #689

@jchmura-sc jchmura-sc self-assigned this Jul 8, 2026
@jchmura-sc

Copy link
Copy Markdown
Collaborator Author

/unit_test

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

GiGL Automation

@ 18:40:24UTC : 🔄 Python Unit Test started.

@ 19:45:22UTC : ✅ Workflow completed successfully.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

GiGL Automation

@ 18:40:26UTC : 🔄 Scala Unit Test started.

@ 18:50:44UTC : ✅ Workflow completed successfully.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

GiGL Automation

@ 18:40:31UTC : 🔄 C++ Unit Test started.

@ 18:42:28UTC : ✅ Workflow completed successfully.

@jchmura-sc jchmura-sc marked this pull request as ready for review July 8, 2026 19:57

@kmontemayor2-sc kmontemayor2-sc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the work Jacob! I would appreciate if we can test this at scale, is it possible to do so and follow up offline?

Comment thread gigl/distributed/base_sampler.py Outdated
Comment on lines +285 to +286
futs = {}
label_keys = set()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. can we type these? e.g. futs: dict[str, torch.futures.Future[Tensor] = {}

Comment thread gigl/distributed/base_sampler.py Outdated
Comment on lines +363 to +364
futs = {}
label_keys = set()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do two sets of rpc send / gather here?

would it be easiest if we only sent out one batch of RPCs?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, the heterogenous / homogenous are independent but we can simplify this!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrently Issue All dist_feature.async_get()Requests in Base Sampler _collate_fn()

2 participants