Clean unused constructors in pushpull C++/CUDA implementation #8706
Conversation
📝 WalkthroughWalkthroughRefactors pushpull APIs to remove template parameters for Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)monai/csrc/resample/pushpull_cpu.cpp (1)
monai/csrc/resample/pushpull_cuda.cu (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: ytl0623 <[email protected]>
ytl0623
force-pushed
the
fix-unused_constructors
branch
from
cafa41a to
7bd9e9a
Compare
Signed-off-by: ytl0623 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@monai/csrc/resample/pushpull_cpu.cpp`:
- Around line 2190-2202: The PUSHPULL_INSTANTIATE_SOURCE macro emits two
explicit instantiations of the template function pushpull but the second (3-arg)
instantiation is missing one of the bool parameters, so it doesn't match the
actual template signature; update the macro PUSHPULL_INSTANTIATE_SOURCE to
include the same list of bool parameters in the second
template<std::deque<Tensor> pushpull(... )> instantiation so both explicit
declarations for pushpull (the multi-arg and the 3-arg overload) match the
template signature exactly.
In `@monai/csrc/resample/pushpull_cuda.cu`:
- Around line 2152-2164: The PUSHPULL_INSTANTIATE_SOURCE macro produces an
explicit instantiation for pushpull but the second template declaration for the
3-arg overload is missing one bool, so it doesn't match the template signature;
update the macro PUSHPULL_INSTANTIATE_SOURCE to include the missing bool in the
second template std::deque<Tensor> pushpull(...) declaration so both explicit
instantiations' parameter lists exactly match the pushpull template signature
(refer to the macro name PUSHPULL_INSTANTIATE_SOURCE and the function pushpull
to locate and correct the mismatched 3-arg overload).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
monai/csrc/resample/pushpull.hmonai/csrc/resample/pushpull_cpu.cppmonai/csrc/resample/pushpull_cuda.cu
🧰 Additional context used
🧬 Code graph analysis (2)
monai/csrc/resample/pushpull_cpu.cpp (1)
monai/csrc/resample/pushpull_cuda.cu (4)
pushpull(2183-2209)pushpull(2183-2193)pushpull(2214-2241)pushpull(2214-2225)
monai/csrc/resample/pushpull_cuda.cu (1)
monai/csrc/resample/pushpull_cpu.cpp (4)
pushpull(2221-2241)pushpull(2221-2231)pushpull(2246-2267)pushpull(2246-2257)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: build-docs
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: packaging
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (mypy)
🔇 Additional comments (5)
monai/csrc/resample/pushpull_cpu.cpp (2)
38-38: No review needed for comment-only update.
2219-2231: LGTM: vector-ref signatures align with header.Also applies to: 2244-2257
monai/csrc/resample/pushpull_cuda.cu (2)
38-38: No review needed for comment-only update.
2180-2193: LGTM: vector-ref signatures align with header.Also applies to: 2211-2225
monai/csrc/resample/pushpull.h (1)
23-49: API change properly implemented; all internal callers already updated.The signatures in
pushpull.h(lines 29-30, 42-43) correctly requireBoundVectorRefandInterpolationVectorRef. All wrapper functions in the same file (e.g., lines 85-86, 101-102, 125-126) already construct these vector refs from scalar enum values before passing to underlyingcuda::pushpullandcpu::pushpullimplementations. CPU and CUDA implementations are consistent with the header declarations. No breaking changes to external callers detected.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: ytl0623 <[email protected]>
1 participant
Description
pushpull_cpu.cppandpushpull_cuda.cu.BoundType,InterpolationType).BoundVectorRefandInterpolationVectorRefas the only supported input types forboundandinterpolationparameters.PUSHPULL_INSTANTIATEmacros.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.