-
Notifications
You must be signed in to change notification settings - Fork 11
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
added collective communication tutorial #48
Conversation
Hey @sdesrozis , thank you for the comment! I have updated the notebook. |
Does it make sense to add basic examples to illustrate ? (below, copy/paste from >>> # All tensors below are of torch.int64 type.
>>> # We have 2 process groups, 2 ranks.
>>> tensor = torch.arange(2, dtype=torch.int64) + 1 + 2 * rank
>>> tensor
tensor([1, 2]) # Rank 0
tensor([3, 4]) # Rank 1
>>> idist.all_reduce(tensor, op="SUM")
>>> tensor
tensor([4, 6]) # Rank 0
tensor([4, 6]) # Rank 1 As far I know, there is no example in our doc for What do you think ? Maybe a good answer is no, and add a few examples in the docstring. |
@sdesrozis I have no issue adding these to the tutorial. My initial discussion with @vfdev-5 on this was that we collect all the examples for these functions currently provided in our docs and provide motivation for using them. I can add such basic examples in both the docstrings and here, if needed. What do you think? |
IMO it would be great to have some basic examples in both docstring and here. Anyway, the notebook is great ! |
@Priyansi the cells are not executable right as
? Maybe, we can use |
Hey @vfdev-5 , I made the basic examples executable, could you please re-review them? |
@Priyansi i think using https://pytorch.org/ignite/v0.4.7/distributed.html#ignite.distributed.utils.spawn is more simplier than idist Parallel in this case. What do you think ? |
Yeah this seems simpler, I'll make the change. |
basically, args can be global configuration that user would like to pass into spawn function. In your case, functions are very trivial and do not represent real use-cases... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Priyansi , lgtm
No, I was just wondering if it's necessary to make it required. Anyway, I got it! |
Fixes #16