Skip to content

[cfg] fix: sync strategy from ActorConfig/CriticConfig to EngineConfig #5885

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.

Sign up for GitHub

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

Merged
wuxibin89 merged 1 commit into from
Apr 7, 2026
+8 −0
Merged

[cfg] fix: sync strategy from ActorConfig/CriticConfig to EngineConfig #5885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions verl/workers/config/actor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ def __post_init__(self):
"""Validate FSDP actor configuration parameters."""
super().__post_init__()
self.engine = self.fsdp_config
# Sync strategy to engine config so engine_workers can pick the right FSDP version.
# EngineConfig.strategy defaults to None, so without this, engine_workers.py always
# falls back to FSDP1 even when actor.strategy="fsdp2".
object.__setattr__(self.engine, "strategy", self.strategy)

# backward compatibility
if self.ulysses_sequence_parallel_size > 1:
Expand Down
4 changes: 4 additions & 0 deletions verl/workers/config/critic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ def __post_init__(self):
"""Validate FSDP critic configuration parameters."""
super().__post_init__()
self.engine = self.fsdp
# Sync strategy to engine config so engine_workers can pick the right FSDP version.
# EngineConfig.strategy defaults to None, so without this, engine_workers.py always
# falls back to FSDP1 even when critic.strategy="fsdp2".
object.__setattr__(self.engine, "strategy", self.strategy)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 6, 2026

Choose a reason for hiding this comment

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

high

In addition to syncing the strategy, ulysses_sequence_parallel_size should also be synced to the engine configuration in FSDPCriticConfig for consistency and backward compatibility, similar to the implementation in FSDPActorConfig. Without this, sequence parallelism settings defined at the top level of the critic configuration will not propagate to the underlying FSDP engine.

Note that ulysses_sequence_parallel_size is already defined as a mutable field in FSDPEngineConfig, so direct assignment is permitted.

        object.__setattr__(self.engine, "strategy", self.strategy)

        # backward compatibility
        if self.ulysses_sequence_parallel_size > 1:
            self.fsdp.ulysses_sequence_parallel_size = self.ulysses_sequence_parallel_size


if self.strategy in {"fsdp", "fsdp2"}:
if self.ulysses_sequence_parallel_size > 1:
Expand Down
Loading