Skip to content

Fix optimizer state restoration and multi-host sharding in WAN trainer - #484

Open
ninatu wants to merge 1 commit into
mainfrom
ninatu/fix_wan_optimizer
Open

ninatu wants to merge 1 commit into
mainfrom
ninatu/fix_wan_optimizer

Conversation

@ninatu

@ninatu ninatu commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Previously, optimizer state restoration was silently failing as a no-op because state.replace(opt_state=...) returns a new TrainState rather than mutating in place, and its return value was discarded.

Fixing the assignment exposed that the restored optimizer state had no device sharding applied:

  1. Restored optimizer state from checkpoints must be explicitly sharded across the device mesh (state_shardings.opt_state) to run on device.

  2. Because Orbax checkpoint key paths diverge from live Optax PyTree structures (string indices vs. JAX KeyEntry objects and trailing .value wrappers), restore_optimizer_state_by_path was added to map restored checkpoint leaves onto the target sharded PyTree structure.

  3. In max_utils.device_put_replicated, populating device shards from host arrays via jax.make_array_from_callback caused eager cross-process coordination failures across multi-host setups. Converting input arrays to local NumPy arrays (np.asarray(arr)) allows clean, process-local slicing per device shard without inter-host dispatch.

- Fix no-op state.replace(...) when restoring opt_state and step by reassigning state.
- Add restore_optimizer_state_by_path to normalize and align Orbax checkpoint paths with live Optax PyTree structure.
- Update max_utils.device_put_replicated to convert to NumPy host arrays before callback slicing to avoid multi-host dispatch failures, and fast-path matching shardings.
@ninatu
ninatu requested a review from entrpn as a code owner September 16, 2026 17:33
@github-actions

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors optimizer state restoration in base_wan_trainer.py by matching parameter paths and applying sharding, improves checkpoint saving logic, and optimizes array replication in max_utils.py. Feedback recommends avoiding synchronous device-to-host transfers in max_utils.py by using jax.device_put directly on JAX arrays, and simplifying the PyTree flattening logic in base_wan_trainer.py by using jax.tree_util.tree_flatten_with_path universally.

Comment on lines 509 to +512
arr = getattr(x, "value", x)
arr_np = np.asarray(arr)
shd = getattr(sharding, "value", sharding)
res = jax.make_array_from_callback(arr.shape, shd, lambda index: arr[index])
res = jax.make_array_from_callback(arr_np.shape, shd, lambda index: arr_np[index])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Calling np.asarray(arr) on a jax.Array triggers a synchronous device-to-host transfer, which blocks the CPU and degrades performance. Instead, if arr is already a jax.Array, we can directly use jax.device_put(arr, shd) to perform an efficient in-device or cross-device/host resharding without copying the data back to the host CPU.

  arr = getattr(x, \"value\", x)\n  shd = getattr(sharding, \"value\", sharding)\n  if isinstance(arr, jax.Array):\n    res = jax.device_put(arr, shd)\n  else:\n    arr_np = np.asarray(arr)\n    res = jax.make_array_from_callback(arr_np.shape, shd, lambda index: arr_np[index])

Comment on lines +83 to +88
if isinstance(restored_opt_state, (dict, flax.core.FrozenDict)):
flat_restored = flax.traverse_util.flatten_dict(restored_opt_state)
raw_items = flat_restored.items()
else:
leaves_with_path, _ = jax.tree_util.tree_flatten_with_path(restored_opt_state)
raw_items = leaves_with_path

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Instead of branching on isinstance(restored_opt_state, (dict, flax.core.FrozenDict)) and using flax.traverse_util.flatten_dict, we can use jax.tree_util.tree_flatten_with_path for all PyTree structures. Since _normalize_path already correctly handles both raw strings/integers and JAX KeyEntry objects (like DictKey), this simplifies the code and makes it robust to any PyTree container type.

  raw_items, _ = jax.tree_util.tree_flatten_with_path(restored_opt_state)

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.

1 participant