Repository navigation
fix(rewards): reject reversed, empty and non-finite boxes in bbox_giou_reward - #4872
Open
jimmy-holdout wants to merge 1 commit into
Open
jimmy-holdout wants to merge 1 commit into
jimmy-holdout wants to merge 1 commit into
Conversation
…u_reward bbox_giou_reward did not check the response box. With reversed corners the "area" is positive and the intersection empty, so the hull can collapse to the ground-truth box and the reward exceeds 1 (a constant [1000, 1000, 0, 0] averages 7.03 over RefCOCO val). NaN or inf coordinates, or finite ones large enough to overflow the areas, give NaN, inf or huge rewards. Return 0 for those boxes so the reward stays in [-1, 1]. Signed-off-by: jimmy-holdout <346065422+jimmy-holdout@users.noreply.github.com>
This branch has not been deployed
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 ?
Return 0 from
bbox_giou_rewardwhen the response box has reversed corners, zero area, non-finite coordinates or coordinates large enough to overflow the area, so the reward stays in [-1, 1] for a well-formed ground truth.Issues
Closes #4871
Usage
No API change. A response box must have finite coordinates with
x1 < x2andy1 < y2, as the refcoco prompt already asks.Behaviour change: the gold box written with its corners swapped (
[x2, y2, x1, y1]) now scores 0 instead of 1. The prompt defines x1, y1 as the top left, so I treated reversed corners as a malformed answer rather than sorting them. Sorting would also keep the reward bounded, if you'd prefer that.Rejected boxes get 0, the same as an unparseable answer today, which is above the GIoU of a badly placed well-formed box (down to -1). Returning -1 instead would remove that gap; I kept 0 for consistency with the existing parse-failure path.
Before your PR is "Ready for review"
Pre checks:
Additional Information
Two new tests in
tests/unit/rewards/test_rewards.py:test_bbox_giou_reward_rejects_invalid_boxescovers reversed corners, zero width, NaN, inf and area overflow.test_bbox_giou_reward_is_boundedchecks 2,000 random response boxes against valid ground truth.mainand pass here.tests/unit/rewards/test_rewards.pypasses in full (9 tests); no functional test exercises this function. A fuzz run of 150,000 random response boxes (including magnitudes up to 1e300) against valid ground truths gave no reward outside [-1, 1].ruff checkandruff format(v0.9.9, as pinned in pre-commit) are clean.RefCOCO validation (3,811 items), each answer wrapped in a full
<think>…</think> <answer>…</answer>response and scored throughVLMVerifyWorkerwith theenv.refcocoblock ofvlm_grpo_3B.yaml, before and after:[1000, 1000, 0, 0][nan, nan, nan, nan]Docs: no page covers the reward functions, so only the docstring is updated.
Unit tests only: no functional test exercises this reward function, so that checkbox is left unticked. Happy to share the RefCOCO scoring script.