[https://nvbugs/6162328][fix] Add a tiny compat shim in load_hf_tokenizer that, when bytes_to_unicode is m#14090
Conversation
📝 WalkthroughWalkthroughThis PR adds a compatibility shim to restore the missing ChangesGPT-2 bytes_to_unicode Compatibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/tokenizer/tokenizer.py (1)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd/update the required NVIDIA copyright header on this modified source file.
This file was modified, but the required header/year update is missing in the provided content.
As per coding guidelines, "All TensorRT-LLM source files must contain an NVIDIA copyright header with year of latest modification, using the Apache 2.0 license text".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/tokenizer/tokenizer.py` at line 1, Add the required NVIDIA copyright header (with the year of latest modification) and Apache 2.0 license block to the top of tokenizer.py (above the existing importlib statement); ensure the header text matches the project's standard NVIDIA copyright + Apache-2.0 boilerplate and the year reflects the most recent change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tensorrt_llm/tokenizer/tokenizer.py`:
- Line 1: Add the required NVIDIA copyright header (with the year of latest
modification) and Apache 2.0 license block to the top of tokenizer.py (above the
existing importlib statement); ensure the header text matches the project's
standard NVIDIA copyright + Apache-2.0 boilerplate and the year reflects the
most recent change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d4fa2bf4-75f7-4aef-b610-3adc4df316d0
📒 Files selected for processing (1)
tensorrt_llm/tokenizer/tokenizer.py
a9e9bf7 to
8600751
Compare
…tokenizers on transformers 5.x Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
8600751 to
6f44f6e
Compare
|
/bot run --skip-test |
|
PR_Github #50191 [ run ] triggered by Bot. Commit: |
|
PR_Github #50191 [ run ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #50242 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #50242 [ reuse-pipeline ] completed with state |
…nizer` that, when `bytes_to_unicode` is m (NVIDIA#14090) Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com> Co-authored-by: xinhe-nv <200704525+xinhe-nv@users.noreply.github.com>
3 participants
Summary
trust_remote_codetokenizer (tokenization_kimi.py) importsbytes_to_unicodefromtransformers.models.gpt2.tokenization_gpt2, but transformers 5.3.0 removed that re-export, so tokenizer load fails andllm.tokenizerbecomes None.load_hf_tokenizerthat, whenbytes_to_unicodeis missing on the gpt2 module, copies it back fromtransformers.convert_slow_tokenizer(where it still lives). No-op on older transformers.Test plan
Links
Summary by CodeRabbit