refactor(autoware_tensorrt_plugins): remove SegmentCSR nD indptr path#12739
Conversation
d52b627 to
5ac0683
Compare
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
5ac0683 to
b93af83
Compare
9eb3e10 to
d603e63
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the SegmentCSR TensorRT plugin and its CUDA kernel launcher to remove the previously-unreachable n-dimensional indptr/size-vector code path, aligning the implementation with the plugin’s documented 2D src + 1D indptr contract.
Changes:
- Simplifies
segment_csr_launchAPI fromstd::vector<int32_t>size parameters to explicit(num_rows, num_cols, indptr_size)integers. - Removes nD
indptrbookkeeping utilities (includingindptr_to_offset) and related kernel indexing paths. - Updates plugin enqueue code and unit tests to use the new launcher signature.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| perception/autoware_tensorrt_plugins/test/segment_csr_kernel_test.cpp | Updates kernel test calls to the new segment_csr_launch signature. |
| perception/autoware_tensorrt_plugins/src/segment_csr_plugin.cpp | Updates plugin enqueue logic to pass (num_rows, num_cols, indptr_size) directly. |
| perception/autoware_tensorrt_plugins/src/scatter_ops/segment_csr.cu | Removes nD launch path, simplifies kernels/launch signature, and deletes nD offset logic. |
| perception/autoware_tensorrt_plugins/include/autoware/scatter_ops/utils.cuh | Removes the now-dead indptr_to_offset helper. |
| perception/autoware_tensorrt_plugins/include/autoware/scatter_ops/segment_csr.h | Replaces vector-based API with scalar size args and adds/updates contract documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12739 +/- ##
===========================================
- Coverage 19.58% 0.00% -19.59%
===========================================
Files 1903 41 -1862
Lines 131340 1587 -129753
Branches 45785 0 -45785
===========================================
- Hits 25718 0 -25718
+ Misses 84661 1587 -83074
+ Partials 20961 0 -20961
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Remove the unused n-dimensional indptr launcher path and offset helper now that the plugin contract is explicitly 1D. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
d603e63 to
0f14e0a
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Third PR in the SegmentCSR split stack.
This PR removes the unreachable n-dimensional
indptrlauncher path. The TensorRT plugin already requires a 2D source tensor and 1Dindptr, so the vector-shaped launcher,indptr_to_offsethelper and bookkeeping are dead code.Since the
SegmentCSRplugin has been introduces for PTv3, which isn't using the nD case, and since there are no other users, removing seems like the best option.Stack
indptrpath.