use CCE fix for TP using vocab parallel for CEL#3000
Conversation
📝 WalkthroughWalkthroughThis change updates the commit hash for the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/cutcrossentropy_install.py (1)
30-33: Avoid future hash drift by centralising the commit constantThe same hash string now appears in several places (README, this script,
__init__.py, notebooks). Keeping them in sync is error-prone. Consider defining a single constant, e.g. insrc/axolotl/integrations/cut_cross_entropy/_version.py, and importing it here:-from packaging.version import Version as V +from packaging.version import Version as V +from axolotl.integrations.cut_cross_entropy._version import CCE_COMMIT ... - + f'{UV_PREFIX}pip install "cut-cross-entropy[transformers] @ git+https://github.com/axolotl-ai-cloud/ml-cross-entropy.git@cbd58e0"' + + f'{UV_PREFIX}pip install "cut-cross-entropy[transformers] @ git+https://github.com/axolotl-ai-cloud/ml-cross-entropy.git@{CCE_COMMIT}"'This prevents silent divergence the next time the hash needs changing.
src/axolotl/integrations/cut_cross_entropy/__init__.py (1)
35-38:_CCE_INSTALL_MESSAGEduplicates logic found elsewhere
cbd58e0is hard-coded here as well as in the install script and docs. If you adopt a shared constant as suggested above, refactor this message to interpolate the same value:-from ._version import CCE_COMMIT -_CCE_INSTALL_MESSAGE = ( - "Please install Axolotl's fork of cut_cross_entropy with transformers support using " - f'`pip install "cut-cross-entropy[transformers] @ git+https://github.com/axolotl-ai-cloud/ml-cross-entropy.git@{CCE_COMMIT}"`' -) +from ._version import CCE_COMMIT +_CCE_INSTALL_MESSAGE = ( + "Please install Axolotl's fork of cut_cross_entropy with transformers support using " + f'`pip install "cut-cross-entropy[transformers] @ git+https://github.com/axolotl-ai-cloud/ml-cross-entropy.git@{CCE_COMMIT}"`' +)This single-source-of-truth pattern will keep documentation, runtime checks, and helper scripts aligned.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/colab-notebooks/colab-axolotl-example.ipynb(1 hunks)scripts/cutcrossentropy_install.py(1 hunks)src/axolotl/integrations/cut_cross_entropy/README.md(1 hunks)src/axolotl/integrations/cut_cross_entropy/__init__.py(1 hunks)
🔇 Additional comments (2)
src/axolotl/integrations/cut_cross_entropy/README.md (1)
22-22: Verify the new commit hash actually exists and is publicly accessibleHard-coding a commit reference protects against breaking changes upstream, but only if the hash is valid in the remote repository. Please double-check that
cbd58e0is pushed toaxolotl-ai-cloud/ml-cross-entropyand still contains the expected CCE fix; otherwise the install command will fail at runtime.examples/colab-notebooks/colab-axolotl-example.ipynb (1)
40-44: Commit hash bump is consistent with the rest of the repo – LGTM
The notebook now installscut-cross-entropy[transformers]from commitcbd58e0, matching the other updated installation references. No further action required.
|
📖 Documentation Preview: https://688ceaa93a1848ee054b0815--resonant-treacle-0fd729.netlify.app Deployed on Netlify from commit 608bdff |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Description
We need to use Vocab Parallel for CCE with TP
Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit