Skip to content

Conversation

@stas00
Copy link
Collaborator

@stas00 stas00 commented Aug 6, 2025

currently passing deepspeed ... --venv_script foo.sh ends up with a pdsh cmd like:

pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh export NCCL_NET_PLUGIN=blah; ...

you can see, ; is missing before exports start, so the first export is ignored.

It should be:

pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh; export NCCL_NET_PLUGIN=blah; ...

This PR is fixing it.

@stas00 stas00 requested a review from loadams as a code owner August 6, 2025 17:21
@stas00 stas00 requested review from tjruwase and removed request for loadams August 6, 2025 17:21
@stas00 stas00 enabled auto-merge (squash) August 8, 2025 14:52
@stas00 stas00 merged commit 8e02992 into master Aug 11, 2025
8 checks passed
@stas00 stas00 deleted the stas00-patch-1 branch August 11, 2025 19:12
LYMDLUT pushed a commit to LYMDLUT/DeepSpeed that referenced this pull request Aug 20, 2025
currently passing `deepspeed ... --venv_script foo.sh` ends up with a
pdsh cmd like:

```
pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh export NCCL_NET_PLUGIN=blah; ...
```
you can see, `;` is missing before exports start, so the first export is
ignored.

It should be:

```
pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh; export NCCL_NET_PLUGIN=blah; ...
```

This PR is fixing it.

Signed-off-by: lym <[email protected]>
mauryaavinash95 pushed a commit to DataStates/DeepSpeed that referenced this pull request Oct 4, 2025
currently passing `deepspeed ... --venv_script foo.sh` ends up with a
pdsh cmd like:

```
pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh export NCCL_NET_PLUGIN=blah; ...
```
you can see, `;` is missing before exports start, so the first export is
ignored.

It should be:


```
pdsh -S -f 1024 -w 10.4.11.15,10.4.10.1 source foo.sh; export NCCL_NET_PLUGIN=blah; ...
```

This PR is fixing it.
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.

4 participants