-
Notifications
You must be signed in to change notification settings - Fork 696
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
feat(tensorrt_common): add gtest #5174
feat(tensorrt_common): add gtest #5174
Conversation
Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5174 +/- ##
==========================================
- Coverage 15.32% 14.89% -0.44%
==========================================
Files 1721 1627 -94
Lines 118559 112358 -6201
Branches 37995 34643 -3352
==========================================
- Hits 18169 16734 -1435
+ Misses 79657 76868 -2789
+ Partials 20733 18756 -1977
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This pull request has been automatically marked as stale because it has not had recent activity. |
@@ -16,6 +16,45 @@ if(NOT (CUDAToolkit_FOUND AND CUDNN_FOUND AND TENSORRT_FOUND)) | |||
return() | |||
endif() | |||
|
|||
# Download onnx |
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.
We decided not to include download scripts in CMake.
Maybe we could skip TestGetInputDims for now.
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.
Without downloading the model this package will not be able to be tested.
so do I go ahead and write test case for this package?
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.
To be honest, this package is not really used in autoware.
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.
Are you sure about that? The header files seems to be included by the other packages. (example)
I will discuss this in the next Software WG if we could consider adding sample models in the repository or not for tests.
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.
To be honest, this package is not really used in autoware.
@mitsudome-r
sorry, I didn't make it clear, there are 3 packages in the perception folder related to tensorrt: tensorrt_classifier
, tensorrt_yolo
, tensorrt_yolox
, where both tensorrt_classifier
and tensorrt_yolox
contain the tensorrt_common
, but tensorrt_yolo
does not contain it. for our real car tests we only used the tensorrt_yolo
.
This pull request has been automatically marked as stale because it has not had recent activity. |
Description
add gtest to tensorrt_common
Tests performed
unit test
Effects on system behavior
There is no effect on the function.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.