Why pass size["longest_edge"] to max_pixels, and size["shortest_edge"] to min_pixels when doing smart_resize
Author: leijie-ww
Pull request: no

@yonigozlan Hi, could you plz help explain why these code is written as follows:

file: transformers\models\qwen2_vl\image_processing_qwen2_vl_fast.py

    def _preprocess(
        self,
        images: list["torch.Tensor"],
        do_resize: bool,
        size: SizeDict,  ...
    ):
        for shape, stacked_images in grouped_images.items():
            height, width = stacked_images.shape[-2:]
            if do_resize:
                resized_height, resized_width = smart_resize(
                    height,
                    width,
                    factor=patch_size * merge_size,
                    min_pixels=size["shortest_edge"],          -- is shortest_edge equal to min_pixels? thanks
                    max_pixels=size["longest_edge"],
                )

Comments:
Hello! `min_pixels` and `max_pixels` are supported for backward compatibility, but users should set `shortest_edge` and `longest_edge` in the `size` dict, which corresponds respectively to `min_pixels` and `max_pixels`.
However setting shortest_edge/min_pixels or longest_edge/max_pixels should result in the same behavior

> Hello! `min_pixels` and `max_pixels` are supported for backward compatibility, but users should set `shortest_edge` and `longest_edge` in the `size` dict, which corresponds respectively to `min_pixels` and `max_pixels`. However setting shortest_edge/min_pixels or longest_edge/max_pixels should result in the same behavior

Just to clarify, the original intent for `smart_resize()`, as I understand it, was to constrain the total image area (i.e., minimum and maximum number of pixels), but in the current usage it’s effectively being repurposed to enforce minimum and maximum edge lengths, correct?

From what I understand, newer vision models are moving toward edge-length-based resizing as a standard convention. Would it make sense to refactor `smart_resize()` to update the parameter names or add clearer aliases for consistency?

I’d be happy to open a PR for that if it’s something you’d consider 😄 


Hi @7amim ! Not sure when this change was done, but you're completely right, the naming here (using shortest_edge/longest_edge instead of min_pixels/max_pixels) is confusing and not accurate. The best solution here would have been to add support for min/max_pixels in the size dictionary, with better naming (min_area/max_area). We can try to transition to min_area/max_area in size, but it would be a deprecation on top of a deprecation, + as you said, new models don't seem to be using this anymore, so not sure if it's worth the trouble...

> Hi [@7amim](https://github.com/7amim) ! Not sure when this change was done, but you're completely right, the naming here (using shortest_edge/longest_edge instead of min_pixels/max_pixels) is confusing and not accurate. The best solution here would have been to add support for min/max_pixels in the size dictionary, with better naming (min_area/max_area). We can try to transition to min_area/max_area in size, but it would be a deprecation on top of a deprecation, + as you said, new models don't seem to be using this anymore, so not sure if it's worth the trouble...

Hi @yonigozlan Thank you very mcuh for your reply.
I am still bit confused, just to make myself more clear :-)
In smart_resize, min_pixels/max_pixels are processed as total number of pixles, while in _preprocess, the min_pixels/max_pixels are passed with size["shortest_edge"] and size["longest_edge"].

If model/preprocessor_config.json is configured as below:
    "size": {
        "longest_edge": 16777216,
        "shortest_edge": 65536
    }
Should we set max_pixel = size["longest_edge"] * H or W * size["longest_edge"], while calling smart_resize? Or do we need refactor smart_resize to support passing longest_edge/shortest_edge as parameter? 
Plz correct me if I misunderstand here. Thank you

> > Hi [@7amim](https://github.com/7amim) ! Not sure when this change was done, but you're completely right, the naming here (using shortest_edge/longest_edge instead of min_pixels/max_pixels) is confusing and not accurate. The best solution here would have been to add support for min/max_pixels in the size dictionary, with better naming (min_area/max_area). We can try to transition to min_area/max_area in size, but it would be a deprecation on top of a deprecation, + as you said, new models don't seem to be using this anymore, so not sure if it's worth the trouble...
> 
> Hi [@yonigozlan](https://github.com/yonigozlan) Thank you very mcuh for your reply. I am still bit confused, just to make myself more clear :-) In smart_resize, min_pixels/max_pixels are processed as total number of pixles, while in _preprocess, the min_pixels/max_pixels are passed with size["shortest_edge"] and size["longest_edge"].
> 
> If model/preprocessor_config.json is configured as below: "size": { "longest_edge": 16777216, "shortest_edge": 65536 } Should we set max_pixel = size["longest_edge"] * H or W * size["longest_edge"], while calling smart_resize? Or do we need refactor smart_resize to support passing longest_edge/shortest_edge as parameter? Plz correct me if I misunderstand here. Thank you

Hi @leijie-cn, If I may chip in based on my understanding, and of course I am open to correction:

smart_resize() operates on proportional comparisons between image size and the provided bounds. It expects area-like quantities, but will still behave predictably for any monotonic scalar measure of image scale (e.g., edge lengths), even though the comparison is not dimensionally consistent.

In other words, whether those bounds represent total area or side length, the same proportional math (based on β = √(target/current)) holds true. That’s why we can safely pass in size["shortest_edge"] and size["longest_edge"] directly, so no unit conversion is needed.

@yonigozlan Thank you for your response, and sounds good! I’d be happy to explore a small refactor to bring the naming in line with current conventions. My personal goal is to get more familiar with open-source contribution workflows and make it a regular practice. I can draft a PR when possible, and of course, the HF team can decide whether it’s worth merging. 🙂

Thanks [@7amim], understand what you said. what I mean is the code in _preprocess looks not consistent with the code in smart_resize, i.e. h_bar * w_bar > max_pixels, if it's the case, shortest_edge/longest_edge should be configured as pixles, but if someone configure it as edge_length, will the end result be out of expectation

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.