Correct Resize pipeline

[makecent]

Fix #209

[LeoXing1996]

Some dataset configs are influenced by this change, such as lsun-car_pad_512. Please check and fix them as well.

[makecent]

@LeoXing1996 I cannot understand the codes below when there is a -1 in the scale: https://github.com/open-mmlab/mmgeneration/blob/69333cfa4b41768b9d700d2a913e3cfa567e1c9e/mmgen/datasets/pipelines/augmentation.py#L134-L139 https://github.com/open-mmlab/mmgeneration/blob/69333cfa4b41768b9d700d2a913e3cfa567e1c9e/mmgen/datasets/pipelines/augmentation.py#L199-L205 Why don't just simply replace the -1 with a computed size that ensures keeping the ratio, but comparing height and width? My understanding is that you want to only resize the short edge no matter it's the height or width. But this is would be very confusing because: (1) In this case, there will be NO difference between the scale=(-1, 256) and scale=(256, -1). (2) As the experience of using numpy and torch, -1 represents that the length at this specific dimension is automatically computed. I think it would be better to set a new argument for resizing short edges.

Anyway, it should be new topic and this pull request only resolves the "misplaced h and w" and it should work correctly now. Pls check it.

[LeoXing1996]

Yes, in the current code, the output of scale=(-1, 256) is the same as scale=(256, -1), and this means rescaling the shortest edge of the input to 256. Your advice about automatically computing the length of the -1's edge is fair enough. You can open another PR to support this feature.

[makecent]

Yes, in the current code, the output of scale=(-1, 256) is the same as scale=(256, -1), and this means rescaling the shortest edge of the input to 256. Your advice about automatically computing the length of the -1's edge is fair enough. You can open another PR to support this feature.

I am happy that you agree with me. But changing the default operation could be dangerous, and I found that other mmlab repositories, e.g. mmaction2, also use -1 to represent the resizing on short edges. It would be better if your mmlab can get consistent on this. And after that I am willing to help.

[LeoXing1996]

Yes, in the current code, the output of scale=(-1, 256) is the same as scale=(256, -1), and this means rescaling the shortest edge of the input to 256. Your advice about automatically computing the length of the -1's edge is fair enough. You can open another PR to support this feature.

I am happy that you agree with me. But changing the default operation could be dangerous, and I found that other mmlab repositories, e.g. mmaction2, also use -1 to represent the resizing on short edges. It would be better if your mmlab can get consistent on this. And after that I am willing to help.

You are right, openmmlab's repos treat -1 as short edges resize. Therefore I think you can open another issue and PR about how we handle -1 in scale. And in this PR, we only focus on bugs of case 1 and case 2.

[makecent]

Yes, in the current code, the output of scale=(-1, 256) is the same as scale=(256, -1), and this means rescaling the shortest edge of the input to 256. Your advice about automatically computing the length of the -1's edge is fair enough. You can open another PR to support this feature.

I am happy that you agree with me. But changing the default operation could be dangerous, and I found that other mmlab repositories, e.g. mmaction2, also use -1 to represent the resizing on short edges. It would be better if your mmlab can get consistent on this. And after that I am willing to help.

You are right, openmmlab's repos treat -1 as short edges resize. Therefore I think you can open another issue and PR about how we handle -1 in scale. And in this PR, we only focus on bugs of case 1 and case 2.

I solved them in the latest commit, pls check if current version is ok.

[LeoXing1996]

You should run pre-commit install before committing to avoid lint errors.

[makecent]

I rewrited the if-else sentence. But it still cannot pass the pytest because the pytest also need to be updated. Here comes a new question -- when the scale is of float/int or there is a -1 in scale, should we set the keep_ratio to True by default or raise an AssertationError?

[LeoXing1996]

I rewrited the if-else sentence. But it still cannot pass the pytest because the pytest also need to be updated. Here comes a new question -- when the scale is of float/int or there is a -1 in scale, should we set the keep_ratio to True by default or raise an AssertationError?

When scale is float or int, we should assert keep_ratio is True. When -1 in scale and scale is a list, keep_ratio should not be True, because we want to call mmcv.imresize.