fix: prevent zero division in xcycsr_to_xyxy converter - #485
Merged
Borda merged 12 commits intoJul 1, 2026
Conversation
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a numerical stability bug in xcycsr_to_xyxy (center/scale/ratio → xyxy) so degenerate boxes (zero scale or zero aspect ratio) no longer trigger division-by-zero and propagate NaN values into downstream tracking state.
Changes:
- Add an epsilon guard to the
h = scale / wcomputation inxcycsr_to_xyxyfor both 1-D and batch inputs. - Update converter tests to assert finite outputs for degenerate inputs and add a mixed-degenerate batch test case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/trackers/utils/converters.py |
Adds division guard in xcycsr_to_xyxy to prevent NaN/Inf outputs on degenerate boxes. |
tests/utils/test_converters.py |
Updates/extends tests to validate finite outputs for degenerate and mixed batch inputs. |
Borda
changed the title
xcycsr_to_xyxy converter
…additive guard Replace `(w + 1e-6)` with `max(w, 1e-6)` (single-box) and `np.maximum(w, 1e-6)` (batch) so the guard only fires when w=0, leaving normal-box arithmetic exact. Additive form perturbed every decoded height, breaking the existing doctest and causing all 12 CI matrix jobs to fail. - [resolve roboflow#1] Copilot (gh): apply clamped epsilon on single-box path (line 152) - [resolve roboflow#1] Copilot (gh): apply clamped epsilon on batch path (line 165) --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…ove hardcoded epsilon comment Replace exact equality with np.testing.assert_allclose (rtol=1e-6) in test_xcycsr_to_xyxy_zero_aspect so the assertion stays robust across platforms and future epsilon changes. Remove inline comment that hardcoded the 1e-6 constant value. - [resolve roboflow#2] Copilot (gh): approx comparison in zero_aspect assertions - [resolve roboflow#3] /review finding (foundry:qa-specialist): remove epsilon-coupling comment --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
pranaysb
force-pushed
the
fix/converters-zero-division-clean
branch
from
July 1, 2026 13:41
1c66cce to
50ec31a
Compare
…om/pranaysb/trackers into fix/converters-zero-division-clean
…ar type - Add `r1 != 0` guard to `_unfreeze_xcycsr` in ocsort/tracklet.py so zero-aspect boxes yield h=0 (not h=inf), consistent with xcycsr_to_xyxy - Change `else 0.0` to `else np.float64(0.0)` in scalar decode path to eliminate Python float / numpy.float64 type asymmetry --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Parametrize zero_scale + zero_aspect tests into single test_xcycsr_to_xyxy_degenerate_collapses_to_point with ids - Add coordinate value assertions (assert_array_almost_equal) to test_xcycsr_to_xyxy_batch_mixed_degenerate for all 3 rows - Add test_xcycsr_to_xyxy_batch_negative_scale covering NaN propagation in the batch code path - Add test_roundtrip_degenerate_is_lossy asserting known-lossy result for zero-area boxes (width cannot be recovered after scale=0 encoding) - Add pytest.param IDs to all parametrize blocks across both test classes --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda
approved these changes
Jul 1, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a bug in
xcycsr_to_xyxy(src/trackers/utils/converters.py) where bounding boxes with zero scale or zero aspect ratio cause a division by zero, producingNaNvalues that propagate into downstream tracking state.The inverse function,
xyxy_to_xcycsr, already guards this exact division pattern withw / (h + 1e-6). This protection was missing from the reverse conversion. This PR adds the same1e-6epsilon toxcycsr_to_xyxyfor both the single-box and batch paths.Related Issue(s): Fixes #484
Type of Change
Testing
Test details:
Updated
test_xcycsr_to_xyxy_zero_scaleandtest_xcycsr_to_xyxy_zero_aspectto assert finite output instead of NaN, and added a new testtest_xcycsr_to_xyxy_batch_mixed_degeneratecovering a mix of normal and degenerate boxes in one batch call.uv run pytest tests/ -v → 711 passed, 2 skipped
Checklist
Additional Context
Minimal reproducible example showing the bug on unpatched
develop:import numpy as np
from trackers.utils.converters import xcycsr_to_xyxy
box = np.array([10.0, 20.0, 0.0, 1.0])
result = xcycsr_to_xyxy(box)
print(result)
RuntimeWarning: invalid value encountered in scalar divide
[10. nan 10. nan]
(Screenshot attached)
Verified locally:
uv run ruff check— all checks passed.uv run ruff format --check— clean.Screenshot of the ISSUE:-
Issue resolved :
Screenshot— Full test suite passing
'uv run pytest tests/ -v'
Screenshot the final summary line at the bottom (711 passed, 2 skipped).