-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add support for TensorRT v10 (multiple api calls have changed) #11166
Conversation
✅ Deploy Preview for frigate-docs canceled.
|
CC @NateMeyer can you take a look at this? |
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 don't believe I can test TensorRT 10+ since they removed support for my Pascal card. Are we adding this because there will be a TensorRT v10 build of Frigate or will it just be an alternate build/fork that swaps in v10?
When installing Frigate manually, TensorRT might have already been installed with the latest version (v10), hence why I would like to include support. I would like to avoid patching this file everytime I update Frigate. |
To confirm, what do you mean by "installing Frigate manually"? |
We've developed a script at Tteck's Proxmox Helper Script to install Frigate without Docker. For now, it works with CPU and Coral detectors, and I'm working on adding support for TensorRT. See discussion at tteck/Proxmox#2711 |
In general, we wouldn't accept code changes for unsupported install methods. We have rejected PRs in the past for similar reasons. Since this is a community supported detector, if @NateMeyer is ok with the added maintenance burden, then I think it's ok. |
I'm not opposed to having the support for v10 in here, there will come a day we need to upgrade TensorRT to support newer cards. I'm a little unsure of how best to handle it. The current PR is reasonable, but I wonder if we could reduce the duplicate code and refactor it a bit to have the shape, dtype, and input checks happen in separate small functions. |
My suggestion to reduce code duplication would be to check TRT_VERSION before every tensorRT api call that has changed ( I'll work on it later this week |
I was thinking have three new local functions, such as |
…eturn input check (not assertion) in new _binding_is_input function
Updated |
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.
This looks good to me.
Co-authored-by: Nicolas Mowen <nickmowen213@gmail.com>
These changes allow Frigate to use TensorRT 10.x.x models. A few changes going from TensorRT v8 to v10:
binding_is_input
has been removedget_binding_shape
-->get_tensor_shape
get_binding_dtype
-->get_tensor_dtype
And I've added a version check to keep current behavior working for those using TensorRT v7 or v8