Fix load_compress_model retry using bitwise ~ on bool instead of logical not#3857
Open
Chessing234 wants to merge 1 commit intolm-sys:mainfrom
Open
Fix load_compress_model retry using bitwise ~ on bool instead of logical not#3857Chessing234 wants to merge 1 commit intolm-sys:mainfrom
Chessing234 wants to merge 1 commit intolm-sys:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
`fastchat/model/compression.py:load_compress_model` retries tokenizer construction when the first call raises `TypeError` (the "`use_fast=True` is not supported for some models" case called out in the comment). The retry is meant to flip `use_fast` to the opposite value, but it uses the bitwise NOT:
```python
try:
tokenizer = AutoTokenizer.from_pretrained(
model_path, use_fast=use_fast, revision=revision, trust_remote_code=True
)
except TypeError:
tokenizer = AutoTokenizer.from_pretrained(
model_path, use_fast=~use_fast, revision=revision, trust_remote_code=True
)
```
On a Python `bool`, `~` does integer bitwise NOT, not logical negation:
Both are truthy integers. So when the fast path raised `TypeError` (typically because the model's slow tokenizer is the only one available), the fallback still passes a truthy `use_fast=-2` / `use_fast=-1`, which either triggers the same `TypeError` again or silently builds the fast tokenizer, defeating the retry.
Root cause
`~` is the bitwise-not operator; the logical negation of a bool in Python is `not`. The comment directly above the `try` spells out the intent ("`use_fast=True` is not supported for some models"), which is the classic "try fast, fall back to slow" pattern already used verbatim in `fastchat/model/model_adapter.py` — where the equivalent `TypeError` handler uses `use_fast=False`.
Why the fix is correct
Changing `~use_fast` to `not use_fast` makes the fallback actually flip `True → False` / `False → True`, so a slow-tokenizer-only model now loads on retry instead of re-hitting the same failure. Behavior on the success path of the first `from_pretrained` is unchanged; this only affects the one line inside the `except TypeError` branch.