Check server input #2719
Check server input #2719
Conversation
| @@ -219,7 +226,7 @@ def messages2prompt(self, messages, sequence_start=True, **kwargs): | |||
| ret += f'{self.system}{self.meta_instruction}{self.eosys}' | |||
| for message in messages: | |||
| role = message['role'] | |||
| content = message['content'] | |||
| content = get_text(message['content']) | |||
There was a problem hiding this comment.
Is it for the GPT4V messages?
There was a problem hiding this comment.
the content part could be str or ChatCompletionContentPartTextParam
lmdeploy/serve/openai/protocol.py
Outdated
| content: Union[str, List[ContentPartParam]] | ||
|
|
||
|
|
||
| ChatCompletionMessage = Union[SystemMessage, UserMessage, AssistantMessage] |
There was a problem hiding this comment.
I don't think this is the best way to validate them. There are also other roles like tool, and maybe more in the future.
lmdeploy/serve/openai/api_server.py
Outdated
| @@ -334,6 +335,13 @@ async def chat_completions_v1(request: ChatCompletionRequest, | |||
| error_check_ret = await check_request(request) | |||
| if error_check_ret is not None: | |||
| return error_check_ret | |||
| try: | |||
| _ = ChatCompletionRequestMessage(messages=request.messages) | |||
There was a problem hiding this comment.
we can just replace the type hint https://github.com/irexyc/lmdeploy/blob/cc142150780351aebd9dac00539cd74e76d7a369/lmdeploy/serve/openai/protocol.py#L113 . It will check if it is valid itself.
There was a problem hiding this comment.
The messages will converted to list of class if replace the type hint, however the messages2prompt function needs the messages to be list of dict.
|
There are also other roles like |
| @@ -240,7 +240,6 @@ class CompletionRequest(BaseModel): | |||
| stream_options: Optional[StreamOptions] = Field(default=None, | |||
| examples=[None]) | |||
| top_p: Optional[float] = 1.0 | |||
| logprobs: Optional[int] = None | |||
There was a problem hiding this comment.
there is a duplicated logprobs
lmdeploy/model.py
Outdated
| @@ -18,6 +18,15 @@ def random_uuid() -> str: | |||
| return str(uuid.uuid4().hex) | |||
|
|
|||
|
|
|||
| def get_text(content: Union[str, List[dict]]): | |||
| """openai format support Union[str, | |||
| List[ChatCompletionContentPartTextParam]] input.""" | |||
There was a problem hiding this comment.
@irexyc May update the docstring as follows:
"""Within the OpenAI API, the content field may be specified as either a string or a list of ChatCompletionContentPartTextParam (defined in openai). When a list is provided, lmdeploy selects the first element to incorporate into the chat template, as the manner in which OpenAI processes lists is not explicitly defined.
"""
Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily receiving feedbacks. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
Motivation
Please describe the motivation of this PR and the goal you want to achieve through this PR.
Modification
Please briefly describe what modification is made in this PR.
BC-breaking (Optional)
Does the modification introduce changes that break the backward-compatibility of the downstream repositories?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
Use cases (Optional)
If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.
Checklist