fix(llama3): make q/k HF-conversion permute DTensor-safe - #4797
Conversation
Llama3StateDictAdapter._permute / _reverse_permute do a head-splitting view() on the q/k projection weights during HF checkpoint conversion. In the live save/load path those weights are DTensors that FSDP shards along dim 0, the exact dim the view unflattens, so an FSDP degree that does not evenly divide the head count raises "Cannot unflatten unevenly sharded tensor". For llama3-8B (n_kv_heads=8) this breaks HF export above an 8-way FSDP degree, i.e. any typical multi-node run. muse_glimmer's adapter hit the identical failure and fixed it with a local _dtensor_safe decorator (redistribute to Replicate, run the reshape on the full local tensor, redistribute back). Hoist that decorator into the shared protocol module as dtensor_safe, apply it to llama3's two permute helpers, and point muse_glimmer at the shared implementation instead of its own copy. Testing: two new CPU unit tests in test_state_dict_adapter.py. - test_permute_does_not_raise_when_fsdp_degree_exceeds_head_count reproduces the production failure with a fake process group (32-way mesh, 16 heads) and asserts to_hf no longer raises. - test_permute_roundtrip_preserves_dtensor_values_and_placement uses a real gloo process group to confirm the decorator does not alter values or placement in the common case. Both fail on main (the first with the exact production RuntimeError) and pass with this change. Full test_state_dict_adapter.py suite (7 tests) passes; pre-commit hooks (flake8, ufmt, pydoclint, codespell, license headers) are clean on the changed files.
|
Hi @Kaif10! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Claude finished @tianyu-l's task in 5m 3s —— View job Validation of the reported bug
The bug is real
Concrete broken example (llama3-8B
|
|
I see. More questions:
|
|
Claude finished @tianyu-l's task in 1m 43s —— View job Answers1. Lifecycle of the replicated tensor The replicated tensor is transient and scoped to a single replicated = w.redistribute(..., placements=[Replicate()] * mesh.ndim) # all-gather
local = fn(self, replicated.to_local(), *args, **kwargs) # reshape full local
out = DTensor.from_local(local, mesh, [Replicate()] * mesh.ndim, ...)
return out.redistribute(device_mesh=mesh, placements=placements) # reshard backThe last line redistributes back to the original (sharded) placements before returning, and
So for 8B this spike is tiny ( 2. All-to-all instead of all-gather -- yes, this works and is a good optimization The permute only unflattens dim 0 ( Result equivalence holds because permuting a column-shard and then gathering columns is the same as permuting the full tensor -- the row permutation commutes with column sharding. A nice detail: both helpers already default Caveats to weigh before switching:
My recommendation: the current all-gather is a correct, minimal first fix and I'd keep it as-is to unblock 8B multi-node export. The · branch |
Llama3StateDictAdapter._permute/_reverse_permutedo a head-splittingview()on the q/k projection weights during HF checkpoint conversion. In the live save/load path those weights are DTensors that FSDP shards along dim 0, the exact dim the view unflattens, so an FSDP degree that does not evenly divide the head count raisesCannot unflatten unevenly sharded tensor. For llama3-8B (n_kv_heads=8) this breaks HF export above an 8-way FSDP degree, i.e. any typical multi-node run.Fix
muse_glimmer's adapter hit the identical failure and fixed it locally with a_dtensor_safedecorator (redistribute toReplicate, run the reshape on the full local tensor, redistribute back). This hoists that decorator into the sharedtorchtitan/protocols/state_dict_adapter.pyasdtensor_safe, applies it to llama3's two permute helpers, and pointsmuse_glimmerat the shared implementation instead of its own copy.Net diff is +41/-39: llama3 gains two decorators, muse_glimmer loses its 30-line duplicate.
Testing
Two new CPU unit tests in
test_state_dict_adapter.py:test_permute_does_not_raise_when_fsdp_degree_exceeds_head_countreproduces the production failure with a fake process group (32-way mesh, 16 heads) and assertsto_hfno longer raises.test_permute_roundtrip_preserves_dtensor_values_and_placementuses a real gloo process group to confirm the decorator does not alter values or placement in the common (evenly-sharded) case.Both fail on
main— the first with the exact productionRuntimeError— and pass with this change. Fulltest_state_dict_adapter.pyfile passes (7 tests).pre-commit(flake8, ufmt, pydoclint, codespell, license headers) is clean on the changed files.This is a checkpoint-conversion bug fix, not a training-numerics change, so no loss/grad_norm comparison applies.