Question about `calculate_dimensions`

[julien-blanchon]

Hello,

I'm trying to understand the codebase.
I see that you have changed the default behaviour of Qwen Image here

PaCo-RL/PaCo-GRPO/paco_grpo/diffusers_patch/qwenimage_edit_pipeline.py

Line 100 in bc0af8d

calculated_width, calculated_height, _ = calculate_dimensions(max_area, width / height)

In the diffusers implementation 1024 is enforce by default and here we are instead using the resolution we are training on.
I'm also implemented this change on the official flow-grpo codebase to see how it translate and it seems to drastically boost the sampling and training speed, as well as mitigating the common zoom/crop effect when training qwen on resolution that are not 1024.

I'm still waiting to see if the training will actually converge (with flow-grpo + this specific change only) but I'm quite curious to know if you have done abblation on this, and what motivated this change ...

[Jayce-Ping]

Hi there.

it seems to drastically boost the sampling and training speed

The code logic here is to control the image size smaller than the resolution we specified (max_area=train.resolution**2), and keep the aspect ratio at the same time. The resolutions should be consistenct during both sampling and training, and lower than that for final inference. That is our Resolution-decoupled strategy. It should reduce sampling and training cost quadratically.

as well as mitigating the common zoom/crop effect

I don't know this effect actually. This is a windfall! Could you please explain or share some links related?

Additionally, I just noticed that the code logic here is somehow duplicate:

    height: int = config.train.resolution if 'height' not in sample else sample['height'][0] # Given by config.train.resolution
    width: int = config.train.resolution if 'width' not in sample else sample['width'][0] # Given by config.train.resolution
# ....
    max_area = config.train.resolution ** 2
    calculated_width, calculated_height, _ = calculate_dimensions(max_area, width / height)
    height = height or calculated_height # results in Given by `config.train.resolution or Given by config.train.resolution`
    width = width or calculated_width # results in Given by `config.train.resolution or Given by config.train.resolution`

Fountunatly, here, the height and width are enforced to use our specified config.train.resolution or the metadata sample['height']/sample['widht'], as we intended. During our experiment, we tested resolution as 512 and 384 for both training and sampling, consistently. I will correct the logic here in the next commit.

I'm looking forward to your experiments results. I haven't done thourgh ablation due to limited time and resources. Thanks for pointing this out!🤗