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

Fix TensorRT --dynamic excess outputs bug #8869

Merged
merged 5 commits into from
Aug 4, 2022

Conversation

glenn-jocher
Copy link
Member

@glenn-jocher glenn-jocher commented Aug 4, 2022

Potential fix for #8790

πŸ› οΈ PR Summary

Made with ❀️ by Ultralytics Actions

🌟 Summary

Improved TensorRT dynamic input handling in YOLOv5 πŸš€

πŸ“Š Key Changes

  • Renamed dynamic_input variable to dynamic to simplify naming.
  • Modified the logic for reshaping the input if the input image shape does not match the expected model shape.
  • Enhanced error message clarity when an image shape mismatch occurs.

🎯 Purpose & Impact

  • πŸ”¨ Code Maintainability: Renaming the variable makes the code more readable and maintainable.
  • πŸ’‘ Better Error Handling: Clear error messages help to diagnose issues related to input sizes quickly, making it easier for users to debug.
  • ✨ Dynamic Resizing: Enhanced support for dynamic input sizes provides more flexibility for users wanting to process images of various dimensions without manual resizing.

@glenn-jocher glenn-jocher linked an issue Aug 4, 2022 that may be closed by this pull request
1 task
@glenn-jocher
Copy link
Member Author

@democat3457 confirm fix for #8866. Can you please review this PR and see if this approach makes sense?

Screen Shot 2022-08-04 at 8 32 46 PM

@democat3457
Copy link
Contributor

democat3457 commented Aug 4, 2022

I have a better solution, should I make a PR?

Add this line:

...
self.context.set_binding_shape(self.model.get_binding_index('images'), img.shape)
self.bindings['images'] = self.bindings['images']._replace(shape=img.shape)
+ self.bindings['output'].data.resize_(tuple(self.context.get_binding_shape(self.model.get_binding_index('output'))))
...

@glenn-jocher
Copy link
Member Author

@democat3457 got it! I'll add your line and remove my fix.

@glenn-jocher
Copy link
Member Author

glenn-jocher commented Aug 4, 2022

@democat3457 stupid question, is the previous line not modifying inplace? Does it need a reasignment like you have here?

self.bindings['images'] = self.bindings['images']._replace(shape=im.shape)

i.e. could this just be:

self.bindings['images']._replace(shape=im.shape)

@glenn-jocher
Copy link
Member Author

@democat3457 how does this look?

@democat3457
Copy link
Contributor

@democat3457 stupid question, is the previous line not modifying inplace? Does it need a reasignment like you have here?

self.bindings['images'] = self.bindings['images']._replace(shape=im.shape)

i.e. could this just be:

self.bindings['images']._replace(shape=im.shape)

It does not modify in place, the underscore just stops it from getting mixed up with a possible field of the namedtuple. (Also, I looked - there doesn't exist an in-place replacement, sadly)

@democat3457
Copy link
Contributor

@democat3457 how does this look?

Looks good to me!

@glenn-jocher glenn-jocher merged commit 38a6eb6 into master Aug 4, 2022
@glenn-jocher glenn-jocher deleted the fix/trt_dynamic_excess branch August 4, 2022 21:26
@glenn-jocher
Copy link
Member Author

@democat3457 PR is merged. Thank you for your contributions to YOLOv5 πŸš€ and Vision AI ⭐

ctjanuhowski pushed a commit to ctjanuhowski/yolov5 that referenced this pull request Sep 8, 2022
* Fix TensorRT --dynamic excess outputs bug

Potential fix for ultralytics#8790

* Cleanup

* Update common.py

* Update common.py

* New fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

is there some error with dynamic export for TensorRT?
2 participants