Skip to content

[cfg, megatron, doc] fix: honor actor.router_replay or fail if it disagrees with engine - #7466

Open
YeonwooSung wants to merge 1 commit into
verl-project:mainfrom
YeonwooSung:fix/7463-router-replay-dead-key
Open

[cfg, megatron, doc] fix: honor actor.router_replay or fail if it disagrees with engine#7466
YeonwooSung wants to merge 1 commit into
verl-project:mainfrom
YeonwooSung:fix/7463-router-replay-dead-key

Conversation

@YeonwooSung

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #7463.

actor.yaml and the Ascend NPU guide document actor_rollout_ref.actor.router_replay.mode. The Megatron/VeOmni worker only reads actor.{megatron,veomni}.router_replay.mode. Setting the documented key left routing replay disabled.

This PR:

  • Copies a non-disabled actor-level router_replay onto the engine config when the engine is still disabled.
  • Raises if both copies are set to different non-disabled modes.
  • Updates the NPU guide / parameter table to name the engine key as canonical, with the top-level key as a compatibility alias.
  • Makes experimental/separation R2 vs R3 handling read the engine copy first.

Closed #3762 asked whether routing replay would be supported (it is). This is the split-schema no-op, not a request to add the feature.

Checklist Before Starting

Test

pytest tests/workers/config/test_actor_router_replay_sync_on_cpu.py -q

Local machine did not have the full verl test env. Ruff passed on the touched files. Please run the CPU test in CI.

API and Usage Example

# documented key now enables engine-side replay when the engine key is still disabled
actor_rollout_ref.actor.router_replay.mode=R3
actor_rollout_ref.rollout.enable_rollout_routing_replay=True

# still valid, and preferred
actor_rollout_ref.actor.megatron.router_replay.mode=R3

Design & Code Changes

McoreActorConfig / VeOmniActorConfig.__post_init__ call ActorConfig.sync_router_replay_to_engine(). Engine router_replay fields are frozen, so the copy uses object.__setattr__ (same pattern as FSDPActorConfig syncing strategy).

Checklist Before Submitting

  • Read the Contribute Guide.
  • Apply pre-commit checks: pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always
  • Add / Update the documentation.
  • Add unit or end-to-end test(s) to the CI workflow to cover all the code. If not feasible, explain why: CPU unit tests added under tests/workers/config/.
  • Once your PR is ready for CI, send a message in the ci-request channel in the verl Slack workspace.
  • If your PR is related to the recipe submodule, please also update the reference to the submodule commit via git submodule update --remote or cd recipe && git pull origin main.

AI assistance was used to locate the bug and draft the patch. A human author reviewed every changed line.

…agrees with engine

Fixes verl-project#7463

Co-authored-by: Grok <grok@x.ai>
Signed-off-by: YeonwooSung <neos960518@gmail.com>
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.

[config][MoE] actor.router_replay.mode is ignored; official docs point at a no-op key Does verl have plans to support routing replay?

1 participant