Skip to content

feat(annotate_subtasks): only downsample frames larger than target_size#261

Merged
shuheng-liu merged 1 commit into
mainfrom
feat/annotate-subtasks-no-upsample
May 4, 2026
Merged

feat(annotate_subtasks): only downsample frames larger than target_size#261
shuheng-liu merged 1 commit into
mainfrom
feat/annotate-subtasks-no-upsample

Conversation

@WilliamYue37
Copy link
Copy Markdown
Member

What this does

_resize_and_center_crop in src/opentau/scripts/annotate_subtasks.py previously upsampled small frames (e.g. 224×224) up to target_size × target_size (default 448×448) and center-cropped. Upsampling adds no information — it just inflates image-token cost on the Anthropic / Gemini API and slows the request.

This PR changes the behavior so the function:

  • Downsamples + center-crops frames whose shorter side is strictly greater than target_size (unchanged path for typical 30–50 fps recordings shot at >= VGA).
  • Returns frames whose shorter side is <= target_size untouched at their native resolution (no upsample, no crop).

Module docstring, function docstrings, and the --target-size CLI help are updated to match. (🗃️ Feature)

How it was tested

  • Updated tests/scripts/test_annotate_subtasks.py::TestResizeAndCenterCrop:
    • Renamed test_smaller_than_target_is_upscaledtest_smaller_than_target_passes_through and inverted its assertion (224×224 in → 224×224 out).
    • Added test_non_square_smaller_than_target_passes_through (300×400 in → 300×400 out).
    • Added test_short_side_at_target_passes_through_without_cropping (448×600 in → 448×600 out).
  • Existing landscape (640×480) / portrait (480×640) / already-square / arbitrary-target cases still pass — those frames have shorter side > target so the downsample-and-crop path is exercised.
  • pre-commit run --files src/opentau/scripts/annotate_subtasks.py tests/scripts/test_annotate_subtasks.py → all hooks pass.
  • pytest tests/scripts/test_annotate_subtasks.py -m "not gpu" -x → 48 passed.

How to checkout & try? (for the reviewer)

git checkout feat/annotate-subtasks-no-upsample
pytest -sx tests/scripts/test_annotate_subtasks.py::TestResizeAndCenterCrop

Checklist

  • I have added Google-style docstrings to important functions and ensured function parameters are typed.
  • My PR includes policy-related changes.
    • If the above is checked: I have run the GPU pytests (pytest -m "gpu") and regression tests.

🤖 Generated with Claude Code

Frames whose shorter side is already <= target_size now pass through at
their native resolution instead of being upscaled and center-cropped.
Upsampling small frames just inflates image-token cost without adding
information.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

[claude-review] summary for commit 4948551

No blocking issues found.

Verified:

  • Branch logic in _resize_and_center_crop (src/opentau/scripts/annotate_subtasks.py:156-178) matches the docstring: pass-through when short <= target_size, else aspect-preserving downsample + center-crop. Boundary at short == target_size deliberately skips the crop (docstring and test test_short_side_at_target_passes_through_without_cropping agree).
  • Test coverage adequate for the new branches (square smaller, non-square smaller, shorter-side-at-target non-square); existing landscape/portrait/already-square cases still exercise the resize+crop path.
  • No downstream code in annotate_subtasks.py assumes square output — frames flow through _to_jpeg_bytes/_to_b64_jpeg to the API, both PIL JPEG and the Anthropic/Gemini image inputs accept arbitrary aspect ratios.
  • Module docstring, function docstrings, and --target-size CLI help kept in sync with the new behavior.
  • One mild caveat (not a defect): for an input whose shorter side is exactly target_size but is non-square (e.g. 448×600), the new behavior keeps the frame at 448×600 instead of cropping to 448×448, so for that specific shape image-token cost rises slightly vs. before. Common video inputs (≥VGA, where short > target_size) are unaffected, and frames with short < target_size clearly win — overall the PR's stated goal holds.

@WilliamYue37 WilliamYue37 requested a review from akshay18iitg May 4, 2026 23:15
@WilliamYue37 WilliamYue37 self-assigned this May 4, 2026
@shuheng-liu shuheng-liu merged commit bd006de into main May 4, 2026
7 checks passed
@shuheng-liu shuheng-liu deleted the feat/annotate-subtasks-no-upsample branch May 4, 2026 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants