Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correct Resize pipeline #211

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Correct Resize pipeline #211

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b1f972b
Update augmentation.py
makecent Jan 9, 2022
159e5ce
Update augmentation.py
makecent Jan 9, 2022
b0ab0dd
Update lsun-car_pad_512.py
makecent Jan 10, 2022
e98d5b8
Update imagenet_rgb.py
makecent Jan 10, 2022
12568ad
Update imagenet_rgb.py
makecent Jan 10, 2022
41ea798
Update augmentation.py
makecent Jan 10, 2022
1246986
Update augmentation.py
makecent Jan 10, 2022
6a263be
Update augmentation.py
makecent Jan 11, 2022
880e94b
Update augmentation.py
makecent Jan 11, 2022
156d50b
revise pre-commit
makecent Jan 11, 2022
6564429
Update augmentation.py
makecent Jan 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
revise pre-commit
  • Loading branch information
@makecent
makecent committed Jan 11, 2022
commit 156d50b0f08a112d5962710821629de2e5652d78
6 changes: 4 additions & 2 deletions mmgen/datasets/pipelines/augmentation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,16 @@ def __init__(self,
'When max_size is used, '
f'size_factor should also be set. But received {size_factor}.')
if isinstance(scale, float, int):
assert keep_ratio, 'When scale is a single float/int, keep_ratio must be ture'
assert keep_ratio, ('When scale is a single float/int,'
' keep_ratio must be ture')
if scale <= 0:
raise ValueError(f'Invalid scale {scale}, must be positive.')
elif mmcv.is_tuple_of(scale, int):
max_long_edge = max(scale)
max_short_edge = min(scale)
if max_short_edge == -1:
assert keep_ratio, 'When scale includes a -1, keep_ratio must be ture'
assert keep_ratio, ('When scale includes a -1, '
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When -1 in the given scale, we manually calculate the size of image. Therefore we should use mmcv.imresize and keep_ratio should not be True. I wonder if this can pass the unit test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When -1 in the given scale, we manually calculate the size of image. Therefore we should use mmcv.imresize and keep_ratio should not be True. I wonder if this can pass the unit test.

My bad, forget to do the test. But don't you think that here keep_ratio as true is logically correct, since the size is computed according to the ratio?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe setting keep_ratio as True is logically correct because we calculate the image size manually. However, in the current code, we would call mmcv.imrescale when keep_ratio is True. I suggest in this PR, we only fix the bug of size misplace and we can try to refactor the Resize class later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should convert assert keep_ratio to assert not keep_ratio.

'keep_ratio must be ture')
# assign np.inf to long edge for rescaling short edge later.
scale = (np.inf, max_long_edge)
elif scale is not None:
Expand Down