Skip to content

Fix/slime sglang data parallel size - #92

Merged
lyzustc merged 1 commit into
awslabs:mainfrom
jackapbutler:fix/slime-sglang-data-parallel-size
Jul 28, 2026
Merged

Fix/slime sglang data parallel size#92
lyzustc merged 1 commit into
awslabs:mainfrom
jackapbutler:fix/slime-sglang-data-parallel-size

Conversation

@jackapbutler

Copy link
Copy Markdown
Contributor

Problem

SlimeRunner._build_slime_flags() doesn't pass --sglang-data-parallel-size to slime's train.py. Slime versions after commit fa3c990 added validation at sglang_utils/arguments.py:142 that reads args.sglang_data_parallel_size — an attribute that only exists if argparse registers it via the CLI flag. Since SlimeRunner never passed it, argparse never creates the attribute, and validation crashes with AttributeError.

Additionally, the [slime] extra was missing ray as a dependency despite SlimeRunner directly invoking ray start, ray stop, and ray job submit.

Fix

  • Add sglang_data_parallel_size: int = 1 field to the SlimeRunner dataclass
  • Emit --sglang-data-parallel-size unconditionally in _build_slime_flags()
  • Add ray to the [slime] optional dependency group in pyproject.toml

Tests

  • Added two unit tests covering default and custom values for the new flag
  • Full test suite passes (173 tests)
    Additionally this fix was tested for small scale RL experiments using a manual patch internally.

Comment thread pyproject.toml Outdated
# Required by agentcore_rl_toolkit.backends.slime.{gateway,rollout,traces}.
"rllm-model-gateway>=0.1.0",
# SlimeRunner orchestrates training via ray (ray start, ray stop, ray job submit).
"ray",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is not needed? In our installation guide https://awslabs.github.io/agentcore-rl-toolkit/guides/slime-backend-setup/#installation, choose either option A through slime official docker, or option B through install script https://github.com/awslabs/agentcore-rl-toolkit/blob/main/src/agentcore_rl_toolkit/backends/slime/scripts/install_slime.sh, we both need to install slime's dependencies, and ray is included (https://github.com/THUDM/slime/blob/main/requirements.txt#L18)

@lyzustc lyzustc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for contribution! Codes about adding sglang data parallel size look good to me, but I think there is no need to add ray to pyproject.toml as ray is already the dependency of slime package. Also please address merge conflicts in uv.lock.

@jackapbutler
jackapbutler force-pushed the fix/slime-sglang-data-parallel-size branch from 1daf1e0 to 45509ca Compare July 28, 2026 19:08
@jackapbutler

Copy link
Copy Markdown
Contributor Author

Thanks @lyzustc, yes agreed on ray and have removed that commit and rebased on main

@lyzustc
lyzustc merged commit 0e98722 into awslabs:main Jul 28, 2026
2 checks passed
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.

2 participants