Update for transformers 5.0#1436
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis pull request relaxes transformers version constraints across multiple projects, introduces weight initialization and state management improvements for TransformerEngine-backed models, updates weight tying declarations from tuples to typed class variable mappings, and adds TCP port fixture infrastructure to distributed tests for rendezvous configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
20b3b6f to
af11872
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bionemo-recipes/models/esm2/src/esm/convert.py (1)
122-135:⚠️ Potential issue | 🟡 MinorRemove redundant
post_init()call;apply_transforms()already callstie_weights().Testing confirms the current code is safe—weight values are preserved correctly through the
post_init()call (test_convert.py validates roundtrip conversion with strict tolerances). However,apply_transforms()already invokestie_weights()before returning, making the subsequentpost_init()call redundant. Also note thatconvert_esm_hf_to_te()does not callpost_init(), creating an inconsistency.Suggested fix
- output_model.post_init()bionemo-recipes/models/esm2/tests/conftest.py (1)
16-43:⚠️ Potential issue | 🟡 MinorAdd a second blank line after the import block.
Ruff/isort will flag a single blank line before the module-level TRITON env block.
🧹 Suggested fix
-from esm.convert import convert_esm_hf_to_te - -# Fix Triton UTF-8 decoding issue by setting CUDA library path +from esm.convert import convert_esm_hf_to_te + + +# Fix Triton UTF-8 decoding issue by setting CUDA library pathAs per coding guidelines, Follow import sorting configuration as per isort with 2 lines after imports.
🧹 Nitpick comments (3)
bionemo-recipes/models/amplify/pyproject.toml (1)
21-21: Confirm whether AMPLIFY should stay capped at Transformers <5.0.If AMPLIFY is now v5-compatible, aligning this constraint (e.g.,
transformers>=5.0,<6) would prevent users from staying on v4. If not, the TODO is fine but this will keep the package incompatible with v5 installs.💡 Optional alignment with v5 support
- "transformers<5.0", # TODO(BIO-143): update AMPLIFY to support Transformers v5 + "transformers>=5.0,<6", # TODO(BIO-143): update AMPLIFY to support Transformers v5bionemo-recipes/recipes/geneformer_native_te_mfsdp_fp8/requirements.txt (1)
9-9: Add an explicit v5 lower bound to avoid silently running v4.An unconstrained
transformerscan be satisfied by an already-installed v4, which defeats the v5 migration and risks runtime mismatches. Considertransformers>=5.0,<6to lock the supported major.♻️ Suggested constraint
-transformers +transformers>=5.0,<6bionemo-recipes/recipes/esm2_native_te/requirements.txt (1)
11-11: Consider boundingtransformersto the v5 major line.Leaving this unbounded can allow v6+ upgrades with breaking changes. A constrained range still permits v5 updates while preventing accidental major bumps.
🔧 Suggested constraint
-transformers +transformers>=5.0,<6.0
Signed-off-by: Peter St. John <pstjohn@nvidia.com> fix esm2 tests Signed-off-by: Peter St. John <pstjohn@nvidia.com> small llama3 changse Signed-off-by: Peter St. John <pstjohn@nvidia.com> fix esm2 native tests Signed-off-by: Peter St. John <pstjohn@nvidia.com>
15cf66e to
35032bc
Compare
Signed-off-by: Peter St. John <pstjohn@nvidia.com>
Signed-off-by: Peter St. John <pstjohn@nvidia.com>
Signed-off-by: Peter St. John <pstjohn@nvidia.com>
Currently blocked by huggingface/transformers#43510
Summary by CodeRabbit
New Features
Improvements