Skip to content

feat: add FSDP2 for TRL - #66

Open
Neonkraft wants to merge 8 commits into
mainfrom
feat/fsdp2-version
Open

Neonkraft wants to merge 8 commits into
mainfrom
feat/fsdp2-version

Conversation

@Neonkraft

Copy link
Copy Markdown
Collaborator

Summary

Adds FSDP (v1 or v2, via fsdp.version) as an alternative to DeepSpeed for containerized TRL runs. The new fsdp: block is passed to accelerate launch as --fsdp_* flags; null keeps the current behavior.

  • Config: sharding strategy, wrap policy (defaults to the model's _no_split_modules), checkpoint state_dict_type (sharded by default), prefetch, offload, rank-0 loading, and activation checkpointing. The settings go on the launch line, not into TrainingArguments, because TRL loads the model before the Accelerator exists.
  • Validation: submission rejects combinations that would hang, crash, or silently fall back to DDP:
    • deepspeed + fsdp, no container.image (only the container template renders the flags), or backend: llamafactory
    • FSDP2 with NO_SHARD / HYBRID_*, and cpu_ram_efficient_loading without sync_module_states
    • gradient checkpointing without early_stop: false: early stopping makes collectives rank-dependent when sequence lengths differ
    • DPO with Liger Kernel: TRL's Liger DPO loss skips the FSDP root forward
  • Inference checkpoints: the callback now gathers the full state dict on all ranks before rank 0 saves. Before, it only saw the local shards.
  • The review screen shows an FSDP row; sft.yaml / dpo.yaml show the block as a commented alternative.

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Performance
  • Documentation
  • Maintenance

Validation

  • pytest: 229 passed, with new tests for validation, rendered launch flags, and the checkpoint gather.
  • LUMI: DPO + Liger failed under FSDP1 and FSDP2 (FSDP requires a single root module), hence the rejection.

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