-
-
Couldn't load subscription status.
- Fork 10.8k
[Minor] Change warning->warning_once in preprocess #26455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -69,7 +69,7 @@ def get_tokenizer(self) -> AnyTokenizer: | |||||
|
|
||||||
| def get_bos_token_id(self) -> Optional[int]: | ||||||
| if self.tokenizer is None: | ||||||
| logger.warning( | ||||||
| logger.warning_once( | ||||||
| "Using None for BOS token id because tokenizer is not initialized" | ||||||
| ) | ||||||
| return None | ||||||
|
|
@@ -78,7 +78,7 @@ def get_bos_token_id(self) -> Optional[int]: | |||||
|
|
||||||
| def get_eos_token_id(self) -> Optional[int]: | ||||||
| if self.tokenizer is None: | ||||||
| logger.warning( | ||||||
| logger.warning_once( | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this change aligns with other parts of the file, This change, intended to reduce log spam, introduces a crash. The To avoid introducing a crash, I recommend reverting this to
Suggested change
|
||||||
| "Using None for EOS token id because tokenizer is not initialized" | ||||||
| ) | ||||||
| return None | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this change aligns with other parts of the file,
logger.warning_oncewill unfortunately cause a runtimeTypeError. The underlying implementation invllm/logger.pyuses@lru_cacheon a function that takes the logger instance as its first argument. Since logger instances are not hashable, this will cause a crash when this warning is triggered.This change, intended to reduce log spam, introduces a crash. The
*_oncelogging functionality appears to be broken throughout the codebase and needs to be fixed centrally invllm/logger.py.To avoid introducing a crash, I recommend reverting this to
logger.warninguntil the underlying issue withwarning_onceis resolved.