training_extensions icon indicating copy to clipboard operation
training_extensions copied to clipboard

Set mem_cache_img_min_size to input_size if it's none

Open eunwoosh opened this issue 1 year ago • 2 comments

Summary

This PR inlcudes

  • change update_mem_cache_img_max_size to update_mem_cache_img_min_size
  • set update_mem_cache_img_min_size to value of input_size if it's None.
  • set update_mem_cache_img_min_size to None if tiling

Details

change update_mem_cache_img_max_size to update_mem_cache_img_min_size

The reason why trying to resize image smaller before caching is to reduce image save size for mem cache. And it should be bigger than final image size after transforms. If not, some image information can't be distorted or removed. But current update_mem_cache_img_max_size implementation is focusing on maximum input size while keeping image ratio. It means that if image shape is (100, 1000) and update_mem_cache_img_max_size is (200, 200) then image is resized to (20, 200) which can make unexpected result. This PR changes it to guarantee that image isn't resized smaller than update_mem_cache_img_min_size

set update_mem_cache_img_min_size to None if tiling

tiling task needs to tile original images, so if image is resized before caching, it makes problem while tiling data processing.

How to test

Checklist

  • [x] I have added unit tests to cover my changes.​
  • [ ] I have added integration tests to cover my changes.​
  • [ ] I have ran e2e tests and there is no issues.
  • [ ] I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
  • [ ] I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
  • [ ] I have linked related issues.

License

  • [ ] I submit my code changes under the same Apache License that covers the project. Feel free to contact the maintainers if that's a concern.
  • [ ] I have updated the license header for each file (see an example below).
# Copyright (C) 2024 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

eunwoosh avatar Aug 14 '24 08:08 eunwoosh

Could you elaborate why you changed max -> min? max might be semantically right as the cache would downscale if input image is larger than the criteria which is the upper limit.

I thought, but when I applied to set mem_cache_img_max_size to input_size, I changed my thought. I think image shouldn't become smaller than final image size because there is possibility that image information can disappear. But it's not easy to guarantee if max is only choice we have (Please refer what I wrote above). So, I think providing a way to guarantee that is more useful than max.

eunwoosh avatar Aug 23 '24 06:08 eunwoosh

Could you elaborate why you changed max -> min? max might be semantically right as the cache would downscale if input image is larger than the criteria which is the upper limit.

I thought, but when I applied to set mem_cache_img_max_size to input_size, I changed my thought. I think image shouldn't become smaller than final image size because there is possibility that image information can disappear. But it's not easy to guarantee if max is only choice we have (Please refer what I wrote above). So, I think providing a way to guarantee that is more useful than max.

Do you upscale small input image if it's smaller than the mem_cache_img_min_size? The name implies so, but it might be misleading whether it's yes or no.

Cache should generally have 'upper' bound than 'lower' bound. We could have set a right or enough large size according to the final input size.

  • image size: 100x100 / cache max: 1000x1000 / input size: 500x500 -> 100x100 cached
  • image size: 2000x2000 / cache max: 1000x1000 / input size: 500x500 -> 1000x1000 cached
  • image size: 2000x2000 / cache max: 500x500 / input size: 500x500 -> 500x500 cached

In sum, it would be better to change parameter value, not the semantic logic of memcache. But feel free to go ahead with you design choice if others agree :)

goodsong81 avatar Aug 23 '24 06:08 goodsong81