Fix CXCYWH to XYXY conversion for integer bounding boxes#9322
Fix CXCYWH to XYXY conversion for integer bounding boxes#9322zy1git merged 8 commits intopytorch:mainfrom
Conversation
The _cxcywh_to_xyxy function used an incorrect 'ceil trick' for integer division that produced negative coordinates for bounding boxes with odd dimensions. For example, [cx=5, cy=6, w=10, h=13] incorrectly produced [0, -1, 10, 12] instead of [0, 0, 10, 12]. The fix uses float arithmetic (matching torchvision.ops.box_convert) and converts back to the original dtype, ensuring correct results for both integer and floating-point tensors. Fixes pytorch#8887
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9322
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit f85bb20 with merge base 6940e19 ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @raimbekovm! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
zy1git
left a comment
There was a problem hiding this comment.
Left comments for the test part.
|
Thanks for the review! Will update the test name and add the explicit dtype + assertion. |
zy1git
left a comment
There was a problem hiding this comment.
@raimbekovm Just push a solution which works well for the inplace parameter. Feel free to take a look.
|
|
||
| if need_cast: | ||
| cxcywh = cxcywh.to(dtype) | ||
| if inplace: |
There was a problem hiding this comment.
This part can ensure the inplace parameter works well if inplace is True.
|
Thanks @zy1git! I looked through the solution — everything looks correct:
Appreciate your help with this! |
NicolasHug
left a comment
There was a problem hiding this comment.
Thanks for the PR @raimbekovm
|
Hey @zy1git! You merged this PR, but no labels were added. |
Use float arithmetic instead of the ceil trick (div(-2, floor).abs_()) to match the fix applied to _cxcywh_to_xyxy in PR pytorch#9322.
Summary
Fixes incorrect
_cxcywh_to_xyxyconversion that produced negative coordinates for integer bounding boxes with odd dimensions.Bug: The function used a "ceil trick" (
div(-2, floor).abs()) for integer division, which computedceil(h/2)instead of matching the float behavior oftorchvision.ops.box_convert.Example:
[cx=5, cy=6, w=10, h=13](CXCYWH, int64)[0, -1, 10, 12]❌[0, 0, 10, 12]✓Changes
torchvision.ops._box_convert._box_cxcywh_to_xyxy, then convert back to original dtypeFixes #8887
cc @vfdev-5