-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
add option to shorten prompt print in log #991
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
add option to shorten prompt print in log #991
Conversation
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.
Thank you for your contribution! Left some small comments about naming and style
vllm/engine/async_llm_engine.py
Outdated
log_requests: bool = True, | ||
start_engine_loop: bool = True, | ||
max_log_len: int = 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.
Nit: order
log_requests: bool = True, | |
start_engine_loop: bool = True, | |
max_log_len: int = None, | |
log_requests: bool = True, | |
max_log_tokens: int = None, | |
start_engine_loop: bool = True, |
vllm/engine/arg_utils.py
Outdated
parser.add_argument('--max_log_len', | ||
type=int, | ||
default=None, | ||
help='max prompt/token being printed in log') |
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.
Nit:
parser.add_argument('--max_log_len', | |
type=int, | |
default=None, | |
help='max prompt/token being printed in log') | |
parser.add_argument('--max-log-tokens', | |
type=int, | |
default=None, | |
help='max number of prompt tokens being printed in log') |
Please rename max_log_len
to max_log_tokens
in other places. Thanks!
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.
Done for the rename
2a02926
to
a837cd3
Compare
For now, model get big context support, and there is scenario that prompt would be very long, if we also want to kept a eye over the incoming traffic, then there will be huge log created. Signed-off-by: Lei Wen <[email protected]>
a837cd3
to
cf2d0b2
Compare
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.
LGTM! Fixed some minor issues. Thank you for your contribution!
Actually, I changed the name back to "max_log_len" since for prompt string, it limits the number of characters instead of tokens. |
Signed-off-by: Lei Wen <[email protected]> Co-authored-by: Lei Wen <[email protected]> Co-authored-by: Zhuohan Li <[email protected]>
For now, model get big context support, and there is scenario that prompt would be very long, if we also want to kept a eye over the incoming traffic, then there will be huge log created.