Skip to content

Guard the cross-attention FSDP2 wrap for pipeline stage modules - #187

Merged
Naeemkh merged 4 commits into
mainfrom
fix/pp-fsdp2-cross-attention-guard
Aug 26, 2026
Merged

Naeemkh merged 4 commits into
mainfrom
fix/pp-fsdp2-cross-attention-guard

Conversation

@Naeemkh

@Naeemkh Naeemkh commented Aug 26, 2026 •

Copy link
Copy Markdown
Member

Summary

apply_fsdp2 receives a PipelineStageModule under pipeline parallelism, but _fsdp_wrap_transformer_blocks read transformer.cross_attention_layers unconditionally. Only Transformer defines that attribute, so every pp > 1 run died with AttributeError before the first step.

  • Guard the cross-attention FSDP2 wrap with getattr. apply_fsdp2 receives a PipelineStageModule under PP, which has no cross_attention_layers, so every pp > 1 run died with AttributeError before the first step.
  • Widen both signatures to Transformer | PipelineStageModule. With the annotation widened and the guard removed, pyright flags the offending line, so CI catches the next one.
  • Add the first test that calls apply_fsdp2 on a stage module. CPU only, runs in CI today, fails on the unpatched tree.

Testing

  • uv run ruff check kempnerforge/ tests/ passes
  • uv run ruff format --check kempnerforge/ tests/ scripts/ passes
  • uv run pyright kempnerforge/ passes (0 errors)
  • uv run pytest tests/unit/ -v --timeout=60 passes
  • If distributed code changed: uv run torchrun --nproc_per_node=4 -m pytest tests/distributed/ -v
  • If training loop / parallelism / optimizers changed: uv run pytest tests/e2e/ --e2e -v

Closes #185

@codecov

codecov Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
kempnerforge/distributed/parallel.py 65.69% <100.00%> (+6.63%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Naeemkh
Naeemkh requested review from amazloumi and mmshad August 26, 2026 00:48
amazloumi
amazloumi previously approved these changes Aug 26, 2026

@amazloumi amazloumi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. none of comments are blockers.

Comment thread kempnerforge/distributed/parallel.py
Comment thread tests/unit/test_distributed.py Outdated
Comment thread kempnerforge/distributed/parallel.py

@amazloumi amazloumi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the changes.

@Naeemkh
Naeemkh merged commit ccff1b9 into main Aug 26, 2026
6 checks passed
@Naeemkh
Naeemkh deleted the fix/pp-fsdp2-cross-attention-guard branch August 26, 2026 14:15
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.

Pipeline parallelism crashes on startup

2 participants