feat(training_engine): support Raiden-FFI weight synchronization and adopt single synchronizer - #5082
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Raiden weight synchronization in maxtext_engine.py by removing parameter chunking and updating the import path for raiden_synchronizer. It introduces support for FFI-based synchronization under Pathways to prevent client host OOMs and improves error handling by raising explicit exceptions instead of returning empty metadata. Additionally, a test configuration in maxtext_engine_e2e_test.py is updated. Feedback on the changes points out that an error message still references the old import path instead of the newly updated one.
176b50d to
faf29d4
Compare
|
🤖 Hi @igorts-git, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
This pull request introduces valuable improvements to MaxTextTrainingEngine's Raiden weight synchronization by supporting the modern Raiden-FFI device-to-host path under Pathways and moving from a multi-chunk staging model to a single persistent synchronizer. The overall quality of the changes is high, offering a cleaner implementation that mitigates client host OOM risks and proxy transfer timeouts at scale.
🔍 General Feedback
- Simplification: Eliminating multi-chunk splitting and staging significantly simplifies parameter binding and memory management.
- Robustness: Raising explicit
RuntimeErrorandValueErrorexceptions for missing/misconfigured transports or missing_raiden_ffimodules helps fail-fast and provides better diagnostics. - Code Cleanliness: The redundant duplicate initialization of
self._raiden_syncswas properly resolved.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
5c482a2 to
346a621
Compare
|
🤖 Hi @igorts-git, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
This pull request introduces a major optimization to the training engine by integrating Raiden-FFI device-to-host weight synchronization and adopting a single persistent synchronizer. This successfully resolves extreme host RAM pressure, potential OOMs, and transfer timeouts under Pathways scaling. Overall, the implementation is highly focused and architecturally sound.
🔍 General Feedback
- Single Synchronizer Simplification: Eliminating the complex multi-chunking and splitting logic in favor of a single persistent synchronizer is a fantastic simplification that significantly reduces code complexity.
- Fail-fast FFI Enforcement: The explicit validation of
_raiden_ffipresence under Pathways provides highly informative error handling that will save debugging time in incompatible environments. - Test Integrity: The integration of
"scan_layers": Falseinto the post-training E2E unit tests correctly aligns the dummy NNX model's structure with its configuration, preventing unexpected failures.
aca098b to
62426a7
Compare
|
🤖 Hi @igorts-git, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
This PR introduces a significant architectural simplification and memory optimization by transitioning from multi-chunk host CPU weight staging to a single persistent RaidenSynchronizer using Raiden-FFI under Pathways. The overall code changes are highly focused, clean, and greatly improve performance while reducing client RAM pressure under Pathways.
🔍 General Feedback
- Simplification: Removing multi-chunk weight staging and
_split_into_chunks()greatly reduces complexity and code size. - Fail-Fast Error Handling: Emphasizing immediate error reporting on missing imports/FFI is a great developer experience enhancement.
- Resource Cleanup: We provided an inline comment to ensure
release_host_arraysis called withinrelease_weight_sync()for optimal resource/staged-buffer cleanup.
62426a7 to
5ed4358
Compare
5ed4358 to
f3f2a66
Compare
|
Verified this branch end to end on a v5p-8 host (4 chips) running tunix's Identical on both sides, and identical to what the pre-collapse multi-chunk path produced, so the single-synchronizer change is a clean swap on the One note on the payload freeze: @dataclasses.dataclass(frozen=True, kw_only=True)
class RouterReplayTrainerPayload(abstract_engine.TrainerPayload):
The subclass flattens to a single opaque leaf rather than into its fields. It doesn't affect the run above — Qwen3-0.6B is dense, so no router-replay payload is ever constructed — but on an MoE path that passes one across a
|
A9isha
left a comment
There was a problem hiding this comment.
Approved with one comment
76576e6 to
bdd5505
Compare
…adopt single synchronizer - Support Raiden-FFI device-to-host synchronization in MaxTextTrainingEngine. - Enforce Raiden-FFI under Pathways and fail fast if _raiden_ffi is unavailable. - Eliminate multi-chunk staging and adopt a single persistent RaidenSynchronizer. - Clean up lifecycle in release_weight_sync and close. - Set scan_layers=False in maxtext_engine_e2e_test setup_config.
bdd5505 to
de0d099
Compare
Description
This PR integrates Raiden-FFI device-to-host weight synchronization into
MaxTextTrainingEngineand eliminates multi-chunk weight staging in favor of a single persistent synchronizer.This PR depends on the Raiden-FFI integration in Tunix, which has now been merged into
mainvia google/tunix#2059. Certain APIs have change, thus we update the tunix package version to make sure that CI tests are not broken by this PR.Additionally, one needs Pathways images that include the required changes.
I used these custom Pathways images to run:
server:
us-docker.pkg.dev/cloud-tpu-v2-images-dev/pathways/gke/shauryag/unsanitized_server:raiden_20260812proxy_server:
us-docker.pkg.dev/cloud-tpu-v2-images-dev/pathways/gke/shauryag/unsanitized_proxy_server:raiden_20260812Background & Rationale
Under Pathways (
JAX_PLATFORMS=proxy), trainer parameters are proxy-backed. Previously, we synchronized weights by staging them onto client host RAM (host_stage=is_pathways) across multiple sequential chunks (RAIDEN_WEIGHT_SYNC_CHUNKS). At scale, host staging created extreme client RAM pressure, risk of host OOMs, and multi-minute proxy staging timeouts.With Raiden-FFI (
tpu_raiden_jax),RaidenSynchronizerbinds directly to TPU device buffers on Pathways TPU workers without client host CPU staging. This eliminates the need for multi-chunk staging and allows all parameters to be bound and staged in a single operation.Key Changes
is_pathwaysis detected, FFI is enforced and fails fast with an informative error if_raiden_ffiis missing intunix.experimental.weight_sync.raiden_synchronizer.RaidenSynchronizerinstance: Removed_split_into_chunksand multi-chunk staging loops.prepare_weight_sync()now uses a single persistentRaidenSynchronizerinstance, returning a single[metadata]object.release_weight_sync()andclose()to manageself._raiden_syncinstead of iterating over a chunk list.is_pathways or self._raiden_sync.active) to ensure FFI transport initialization and metadata registration, while gracefully bypassing native D2H when running in CPU test environments lacking_ws_lib."scan_layers": Falseintests/post_training/unit/maxtext_engine_e2e_test.py'ssetup_config()so thatprepare_weight_syncdoes not attempt to unscan layers on the 1DDummyNNXModel.Tests
Manual invocation of qwen3.5-0.6B with Pathways trainer using 16 chips and a single rollout worker.
pytest tests/post_training/unit/maxtext_engine_test.py(46/46 passed)pytest tests/post_training/unit/maxtext_engine_constructor_test.py(5/5 passed)pytest tests/post_training/unit/maxtext_engine_e2e_test.py(1/1 passed)pre-commit run --from-ref origin/main --to-ref HEAD(all checks passed: codespell, pylint, pyink)Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.