-
Notifications
You must be signed in to change notification settings - Fork 342
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
remove first empty token check and add input validation testcase #1549
Conversation
zhulinJulia24
commented
May 6, 2024
•
edited
Loading
edited
- add input validate testcases for Add input validation #1525
- remove first empty token check in streaming during the change of Remove first empty chunck for api_server #1527
- add logprobs assert utils for future use
@@ -842,3 +958,124 @@ def test_longtext_input_streaming(self): | |||
assert outputList[0].get('finish_reason') == 'length', outputList | |||
assert outputList[0].get('text') == '' | |||
assert len(outputList) == 1 | |||
|
|||
def test_input_validation(self): |
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.
怎么有两个 test_input_validation
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.
test_input_validation
外层有个class,分别覆盖interactive接口和chat-complation接口
assert_chat_completions_stream_return(outputList[index], | ||
MODEL_NAME) | ||
assert outputList[-1].get('choices')[0].get( | ||
'finish_reason') == 'length' | ||
assert len(outputList) == 102 | ||
assert len(outputList) == 101 |
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.
why do we use such strict rules? we can just encode the output and check if the token number is equal to max_tokens
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.
It make sense. I can chage according this.
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.
There is an endpoint named encode
in api_server. You may try it directly.
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.
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.
There is an endpoint named
encode
in api_server. You may try it directly.
used encode to get token numbers?
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.
I think @AllentDan means assert max_tokens==api_client.encode(response, add_bos=False)[1]
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