Skip to content
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

[Misc]: Can we remove vllm/entrypoints/api_server.py? #3852

Open
hmellor opened this issue Apr 4, 2024 · 11 comments
Open

[Misc]: Can we remove vllm/entrypoints/api_server.py? #3852

hmellor opened this issue Apr 4, 2024 · 11 comments

Comments

@hmellor
Copy link
Collaborator

hmellor commented Apr 4, 2024

Anything you want to discuss about vllm.

While gardening GitHub issues I've seen many issues where the user is using -m vllm.entrypoints.api_server, which indicates that users are either not aware of ignoring the note at the top of the file:

"""
NOTE: This API server is used only for demonstrating usage of AsyncEngine
and simple performance benchmarks. It is not intended for production use.
For production use, we recommend using our OpenAI compatible server.
We are also not going to accept PRs modifying this file, please
change `vllm/entrypoints/openai/api_server.py` instead.
"""

Can we remove it to avoid future confusion?

@hmellor hmellor added the misc label Apr 4, 2024
@FlorianJoncour
Copy link
Contributor

It's probably a good idea.

However, it would probably be better to move it into a subfolder like vllm_example.
On the other hand, I don't think it would be desirable for the OpenAI server to become the only implementation.

It's possible that in the future, it could be useful to create servers implementation of Mistral AI API or Anthropic.

@hmellor
Copy link
Collaborator Author

hmellor commented Apr 4, 2024

We can leave the openai server where it is so that in future if other proprietary APIs are added they can live adjacent to the openai directory.

@hmellor hmellor changed the title [Misc]: Can we remove vllm.entrypoints/api_server.py? [Misc]: Can we remove vllm/entrypoints/api_server.py? Apr 5, 2024
@DarkLight1337
Copy link
Member

DarkLight1337 commented Jun 2, 2024

@simon-mo
Copy link
Collaborator

simon-mo commented Jun 2, 2024

We can fix bug. We just shouldn't add more features to keep it with parity for OpenAI compatibility.

@hmellor
Copy link
Collaborator Author

hmellor commented Jun 2, 2024

Does vllm/entrypoints/api_server.py have any OpenAI compatibility?

@DarkLight1337
Copy link
Member

DarkLight1337 commented Jun 3, 2024

We can fix bug. We just shouldn't add more features to keep it with parity for OpenAI compatibility.

Perhaps we should move it to examples directory as @FlorianJoncour has suggested. Then we can add tests to ensure that the example is working without introducing new features.

@TikZSZ
Copy link

TikZSZ commented Jun 3, 2024

Maybe this could be kept for supporting features in beta/experimental or something that's not covered by open ai api spec.

For example this was the only way I found to send token ids directly to model instead of chat messages, which is helpful incase when people want to experiment with prompts without limitations.

For example mistral v3 function calling requires custom tokenization that isn't covered by chat template is one such example and I think it could be useful debugging other OSS models that come with their own prompt templates that sometimes may not be optimal for whatever reason.

Note: I might be missing the docs regarding sending tokens directly to open ai api spec but I didn't find anything regarding that.

@simon-mo
Copy link
Collaborator

simon-mo commented Jun 3, 2024

Maybe this could be kept for supporting features in beta/experimental or something that's not covered by open ai api spec.

This is a valid point.

Perhaps we should move it to examples directory

We kept the file there for backward compatibility as some people are still using it.

@DarkLight1337
Copy link
Member

DarkLight1337 commented Jun 4, 2024

We kept the file there for backward compatibility as some people are still using it.

How long are we planning to maintain this? I think #4197 / #5090 would provide a good opportunity to drop it. We can also redirect imports/main to the new location of the file.

@DarkLight1337
Copy link
Member

Maybe this could be kept for supporting features in beta/experimental or something that's not covered by open ai api spec.

Correct me if I'm wrong, but it looks like we can't use this file directly to test out those features. If we have to copy this file and modify it elsewhere anyway, I don't see the harm in moving it to the examples and thus adding it to the official documentation.

Copy link

This issue has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this issue should remain open. Thank you!

@github-actions github-actions bot added the stale label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants