-
Couldn't load subscription status.
- Fork 1.7k
test: Add OpenAI frontend testing for LLM API backend #8040
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
fe8081f
be339c5
2f5f06e
c84d215
7026803
d2daa15
d631a64
cab13c3
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 | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||
| # Copyright 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||||||||||||||
| # Copyright 2024-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||
| # Redistribution and use in source and binary forms, with or without | ||||||||||||||||||||||||||
| # modification, are permitted provided that the following conditions | ||||||||||||||||||||||||||
|
|
@@ -31,8 +31,10 @@ | |||||||||||||||||||||||||
| from fastapi.testclient import TestClient | ||||||||||||||||||||||||||
| from tests.utils import OpenAIServer, setup_fastapi_app, setup_server | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ### TEST ENVIRONMENT SETUP ### | ||||||||||||||||||||||||||
| LLMAPI_SETUP = os.environ.get("LLMAPI_SETUP", 0) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def infer_test_environment(): | ||||||||||||||||||||||||||
| # Infer the test environment for simplicity in local dev/testing. | ||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
|
|
@@ -48,7 +50,10 @@ def infer_test_environment(): | |||||||||||||||||||||||||
| import tensorrt_llm as _ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| backend = "tensorrtllm" | ||||||||||||||||||||||||||
| model = "tensorrt_llm_bls" | ||||||||||||||||||||||||||
| if LLMAPI_SETUP: | ||||||||||||||||||||||||||
| model = "tensorrt_llm" | ||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||
| model = "tensorrt_llm_bls" | ||||||||||||||||||||||||||
|
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. Does it make sense to set ex: server/python/openai/tests/test_chat_completions.py Lines 313 to 316 in 205f13c
Not sure what the config.pbtxt 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. The config.pbtxt I thought that the LLM API backend is one of the tensorrtllm backends, so hence didn't have another "llmapi backend" fixture. I think avoiding having an ENV makes more sense, however, right now the logic to determine which backend to use is to import tensorrtllm or vllm module and see if there's an import error, then set the backend and model name accordingly. server/python/openai/tests/conftest.py Lines 47 to 54 in 205f13c
Both general tensorrtllm and llmapi backend cases will import the tensorrtllm module successfully. I think some sort of flags would still be required to know if this is a general trtllm model or a llmapi one. 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.
Do we plan to add a python-based backend like vllm is? 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. Do you mean making it 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. Oh right because they're not using the Will the llmapi 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. Agree - confirming w/ TRT-LLM team on this. |
||||||||||||||||||||||||||
| return backend, model | ||||||||||||||||||||||||||
| except ImportError: | ||||||||||||||||||||||||||
| print("No tensorrt_llm installation found.") | ||||||||||||||||||||||||||
|
|
@@ -57,7 +62,10 @@ def infer_test_environment(): | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def infer_test_model_repository(backend): | ||||||||||||||||||||||||||
| model_repository = str(Path(__file__).parent / f"{backend}_models") | ||||||||||||||||||||||||||
| if LLMAPI_SETUP: | ||||||||||||||||||||||||||
| model_repository = str(Path(__file__).parent / f"{backend}_llmapi_models") | ||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||
| model_repository = str(Path(__file__).parent / f"{backend}_models") | ||||||||||||||||||||||||||
| return model_repository | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
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.
Would be nice to clean up some of the
if/elseeverywhere for llmapi vs trtllm stuff, but hopefully we can consolidate the supported backend implementations once its a bit more mature.