test: Add OpenAI frontend testing for LLM API backend #8040
Conversation
krishung5
changed the title
python/openai/tests/conftest.py
Outdated
| if LLMAPI_SETUP: | ||
| model = "tensorrt_llm" | ||
| else: | ||
| model = "tensorrt_llm_bls" |
There was a problem hiding this comment.
Does it make sense to set backend = "llmapi" and use the backend fixture to check if llmapi in the tests, following similar patterns for vllm/trtllm in the existing tests?
ex:
server/python/openai/tests/test_chat_completions.py
Lines 313 to 316 in 205f13c
Not sure what the config.pbtxt backend field will be for LLM API though - so let me know what you're thinking
There was a problem hiding this comment.
The config.pbtxt backend field will be python for LLM API backend.
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.
The config.pbtxt backend field will be python for LLM API backend.
Do we plan to add a python-based backend like vllm is?
There was a problem hiding this comment.
Do you mean making it backend: llmapi in the config.pbtxt? I don't think there's a plan to do so right now. I could see it be less confusing having this after the pivot is more finalized, right now it's more like adding another python model just like the original one.
There was a problem hiding this comment.
Oh right because they're not using the runtime config.pbtxt feature currently, they just swapped the backend to python instead, gotcha - makes sense.
Will the llmapi model.py replace the existing model.py in trtllm backend? Hopefully we're not planning to support 3 implementations for now 😅
There was a problem hiding this comment.
Agree - confirming w/ TRT-LLM team on this.
python/openai/tests/conftest.py
Outdated
| ### TEST ENVIRONMENT SETUP ### | ||
| LLMAPI_SETUP = os.environ.get("LLMAPI_SETUP", 0) |
There was a problem hiding this comment.
Would be nice to clean up some of the if/else everywhere for llmapi vs trtllm stuff, but hopefully we can consolidate the supported backend implementations once its a bit more mature.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I'll be able to merge this PR only after the LLM API backend MR is merged. Also pending on CI - having some issues with the build which are not related to the openai changes. I'll ask for final review once we're ready to merge. Thanks! |
Marking as draft for clarity - feel free to unmark it when ready |
What does the PR do?
Add OpenAI frontend testing for LLM API backend. Some of the test cases are skipped for now due to issues with the random seeds sampling parameter. Will follow up with the TRT-LLM team.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs: LLM API backend MR.
Where should the reviewer start?
Test plan:
Caveats:
Background
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)