-
Notifications
You must be signed in to change notification settings - Fork 76
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
Adding TRT-LLM config key #825
Conversation
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 overall but would love an explanation of why we have to write the serialization manually. I thought we could rely on pydantic for that
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.
also, I think this PR is missing changes to bump the pydantic version on the server. I thikn that's probably okay assuming all the integration tests pass.
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 left two suggestions that let you drop all the to/from dict functions. lemme know if it works.
@aspctu can you update the example from the description for the most recent interface? |
🚀 What
This PR adds a new key to the
config.yaml
:trtllm
. We use this for models that need engines built or to serve engines. The spec is:This PR also updates
pydantic
to a newer version.💻 How
🔬 Testing
Integration tests passed for pydantic change. Also tested the new key with various configs locally via manual context builds.