[Misc] Make timeout passable in init_distributed_environment #24522
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable timeout for init_distributed_environment, which is a useful addition for scenarios requiring longer initialization times. However, the current implementation has a potential issue where passing the default None value for the timeout to torch.distributed.init_process_group could lead to a runtime error. My review includes suggestions to fix this by conditionally passing the timeout argument, ensuring that the default PyTorch timeout is used when no specific timeout is provided.
vllm/distributed/parallel_state.py
Outdated
There was a problem hiding this comment.
The type hint for timeout should be Optional[timedelta] to correctly reflect that None is a possible value. This improves code clarity and helps static type checkers.
| timeout: timedelta = None, | |
| ): | |
| timeout: Optional[timedelta] = None, | |
| ): |
vllm/distributed/parallel_state.py
Outdated
There was a problem hiding this comment.
Optional[timedelta] as already stated by gemini - otherwise lgtm
jberkhahn
force-pushed
the
timeout_fix
branch
from
f39eb43 to
3048d38
Compare
jberkhahn
force-pushed
the
timeout_fix
branch
from
3048d38 to
6574d10
Compare
njhill
changed the title
jberkhahn
force-pushed
the
timeout_fix
branch
2 times, most recently
from
1aea182 to
b43d7bc
Compare
jberkhahn
force-pushed
the
timeout_fix
branch
from
b43d7bc to
e680723
Compare
jberkhahn
force-pushed
the
timeout_fix
branch
2 times, most recently
from
abd5623 to
e6d6c51
Compare
Signed-off-by: jberkhahn <[email protected]>
jberkhahn
force-pushed
the
timeout_fix
branch
from
e6d6c51 to
64a8cc7
Compare
…oject#24522) Signed-off-by: jberkhahn <[email protected]>
…oject#24522) Signed-off-by: jberkhahn <[email protected]>
…oject#24522) Signed-off-by: jberkhahn <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
…oject#24522) Signed-off-by: jberkhahn <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
4 participants
Purpose
vllm-spyre has been experiencing timeouts on this function in certain scenarios when forcing model compilation to happen serially with multiple backends and large context lengths. This function already has an option to set a timeout, it's just not passed here. This PR makes it configurable, but leaves it as the default (which is 30 minutes).