Support packing for TPU Ulysses attention - #4825
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables sequence packing support for TPU Ulysses context parallelism. It removes the validation checks that previously restricted packing with Ulysses, updates the documentation accordingly, and adds unit tests to verify Ulysses attention works correctly with packed sequences. There are no review comments to address, so I have no additional feedback to provide.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
🤖 Hi @huytransformer, 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 support for sequence packing under TPU Ulysses context parallelism in MaxText. The changes are highly focused, robustly implemented, and include both configuration validation updates and comprehensive mathematical equivalence and gradient equivalence tests.
🔍 General Feedback
- High Code Quality: The implementation is exceptionally clean, with precise removal of the previous constraint and clear updates to the relevant config and train helper files.
- Strong Test Coverage: The tests are well-parameterized and verify both forward output correctness and backward gradient correctness under packed TPU Ulysses configurations.
- Precise Documentation: The documentation in
docs/guides/optimization/sharding.mdwas accurately updated to reflect the new capabilities.
Description
Corollary follow up of PR #4687.
This PR enables sequence packing when using TPU Ulysses attention by lifting the config ban and adding tests.
Tests
tests/unit/attention_test.py -k ulysseson TPU v4: 7 passedtests/unit/configs_value_test.py: Passed.Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.