Keep at least one pixel per axis when resizing an image - #289
juliendenize merged 2 commits into
Conversation
An image whose aspect ratio is more extreme than max_image_size to 1 crashed the encoder on valid input: the resize rounded the shorter side to 0 pixels, the token count for that axis came out 0, and the encoder hit its own assert. A 4 by 10000 image raises AssertionError at the production config today. Both axes now clamp to one pixel, so such an image encodes as a single row or column of tokens.
juliendenize
left a comment
There was a problem hiding this comment.
Thanks for the contribution !
Looks good but could you make parametrized tests and clean up a bit the comments before i approve ?
| # For a very wide or very tall image the resize used to round the shorter side | ||
| # down to 0 pixels, which made the token count for that axis 0 and crashed the | ||
| # encoder on valid input. The shorter side must clamp to at least one token. |
There was a problem hiding this comment.
can be removed imo :)
| # For a very wide or very tall image the resize used to round the shorter side | |
| # down to 0 pixels, which made the token count for that axis 0 and crashed the | |
| # encoder on valid input. The shorter side must clamp to at least one token. |
| ) | ||
| image_encoder = ImageEncoder(image_config, special_token_ids) | ||
|
|
||
| for size in [(1, 512), (4, 10000), (10000, 4), (2, 4096)]: |
There was a problem hiding this comment.
could you use parametrized instead ?
|
Both done, thanks. The four sizes are parametrized now, stacked with the existing Comment block in the test is gone, and I cut the one in All 8 cases still fail on main and pass here, and |
juliendenize
left a comment
There was a problem hiding this comment.
Awesome, thanks for iterating :)
The bug
An image with a very extreme aspect ratio crashes the encoder on valid input. At the production config (
image_patch_size=16,max_image_size=1024), a 4 by 10000 pixel image raises a bareAssertionError:Same for 1 by 4096 and 3 by 8000. Nothing about those images is invalid: they are just tall.
Why
In
_image_to_num_tokens, when the image is larger thanmax_image_sizethe two sides are divided by the ratio and rounded. For a ratio above twice the shorter side,round()takes that side to 0, so the token count for that axis is(0 - 1) // patch + 1, which is 0, and__call__then trips its ownassert w > 0.The change
Both axes clamp to a minimum of one pixel after the resize, so an extremely thin image encodes as a single row or column of tokens instead of crashing. Nothing else changes: any image whose shorter side survives the resize is unaffected, and the existing expectations in
test_image_to_num_tokensstill hold.Tests
test_image_to_num_tokens_extreme_aspect_ratiocovers 1x512, 4x10000, 10000x4 and 2x4096 at bothspatial_merge_sizevalues, asserting both that the axis counts are at least 1 and that the encoder produces the expected token count. It fails onmainat both parameter values and passes here. The rest oftests/test_image.pyis green (22 passed).