Skip to content
This repository was archived by the owner on Dec 1, 2021. It is now read-only.

Conversation

@yd8534976
Copy link
Contributor

@yd8534976 yd8534976 commented Jul 11, 2019

Motivation and Context

Scale augmentation is an important augmentation especially for detection tasks. This PR implements Scale class to support scale augmentation for all existing tasks.

Description

  • Remove affine_scale()
  • Implement Scale class.

How has this been tested?

It is tested well using below notebook.

https://colab.research.google.com/drive/1MvRCKS92LylyhDsx0nIF9Drtt5f97OxZ#scrollTo=Orst-U1mS4rh

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / Optimization (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@yd8534976
Copy link
Contributor Author

yd8534976 commented Jul 11, 2019

I have not addressed a very small box in edge, maybe I should drop them directly. 🤔

@hadusam hadusam added the CI: auto-run Run CI automatically label Jul 12, 2019
@yd8534976
Copy link
Contributor Author

run test

Copy link
Member

@iizukak iizukak left a comment

Choose a reason for hiding this comment

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

@yd8534976 Thank you for PR!
Did you any experiment with Scale augmentation?
I want result of training using and not using training result.

new_image.paste(scaled, (int(outer_width / 2), int(outer_height / 2)))

return np.array(new_image)
class Scale(data_processor.Processor):
Copy link
Member

Choose a reason for hiding this comment

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


return np.array(new_image)
class Scale(data_processor.Processor):
"""Scale.
Copy link
Member

Choose a reason for hiding this comment

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

Why description was removed?

Resize image to scale size keeping the aspect ratio and place it in center of fill color image.

It's obvious?

min_scale, max_scale = scale_range

assert min_scale >= 0.5
assert max_scale <= 1.5
Copy link
Member

Choose a reason for hiding this comment

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

How about add assert with fill_color ?

@yd8534976
Copy link
Contributor Author

Did you any experiment with Scale augmentation?
I want result of training using and not using training result.

@iizukak , Thanks for reviewing! I will fix them. Since I'm training another task.. I'll test this PR later.

Since it will have similar issues as #377 . I worry that they can not support various tasks in future. Actually I want to rewrite them using cv2. 🤔

@iizukak
Copy link
Member

iizukak commented Jul 30, 2019

opencv-python is big and heavy library.
I don't know it's good idea to use OpenCV for augmentation...

@tkng
Copy link
Contributor

tkng commented Jul 30, 2019

Since it will have similar issues as #377 . I worry that they can not support various tasks in future. Actually I want to rewrite them using cv2. 🤔

Now we don't have dependency to cv2 module except some demo scripts. Since OpenCV is very large software, we should think carefully before adding that dependency.

@yd8534976
Copy link
Contributor Author

@tkng

Now we don't have dependency to cv2 module except some demo scripts. Since OpenCV is very large software, we should think carefully before adding that dependency.

Thanks for answering! I think data augmentation will only be used in training environment, does it matter only add them into training dependency?

@yd8534976
Copy link
Contributor Author

yd8534976 commented Jul 30, 2019

@tkng

Or, we can add some functions like this..

https://github.com/dmlc/gluon-cv/blob/c11643d8e81ad08a8c89555a5ad8d4860bb86b19/gluoncv/utils/filesystem.py#L43

def try_import_cv2():
    """Try import cv2 at runtime.
    Returns
    -------
    cv2 module if found. Raise ImportError otherwise
    """
    msg = "cv2 is required, you can install by package manager, e.g. 'apt-get', \
        or `pip install opencv-python --user` (note that this is unofficial PYPI package)."
    return try_import('cv2', msg)

@tkng
Copy link
Contributor

tkng commented Jul 30, 2019

I think data augmentation will only be used in training environment, does it matter only add them into training dependency?

Considering that adding a dependency to OpenCV will increase several hundreds of mega bytes (or maybe over 1 giga bytes) of our docker image size, adding the dependency just for some data augmentation methods seems not a reasonable choice, even only for training environment.

As we already depends on TensorFlow, how about using tf.contrib.image.rotate?

@yd8534976
Copy link
Contributor Author

yd8534976 commented Jul 30, 2019

I think data augmentation will only be used in training environment, does it matter only add them into training dependency?

Considering that adding a dependency to OpenCV will increase several hundreds of mega bytes (or maybe over 1 giga bytes) of our docker image size, adding the dependency just for some data augmentation methods seems not a reasonable choice, even only for training environment.

As we already depends on TensorFlow, how about using tf.contrib.image.rotate?

Sorry, I have never tried tf.contrib.image.rotate... I just worry if it will make graph more complicated and we may need to define a clean name_scope to make it easy to compile.

Well, although rotate augmentation may be helpful, it is not necessary. We may write it by hand or rotate them channel by channel.

@tkng
Copy link
Contributor

tkng commented Jul 31, 2019

I just worry if it will make graph more complicated and we may need to define a clean name_scope to make it easy to compile.

I think we can use tf.contrib.image.rotate in our data augumentation, without affecting our network definition.

BTW, the core of what I wanted to tell here was, before adding a new dependency to large package, we should consider carefully. Otherwise our software will bloat and be hard to use. tf.contrib.image.rotate is just an idea to prevent the bloat, you do not have to be stuck with it.

Well, although rotate augmentation may be helpful, it is not necessary. We may write it by hand or rotate them channel by channel.

Rotate them channel by channel (if channel number != 3) looks a good idea, because it's simple.

@yd8534976
Copy link
Contributor Author

@tkng Okay! I will pay attention to this point. Thanks for your patience!

@yd8534976 yd8534976 changed the title Add scale augmentation for all existing tasks [WIP] Add scale augmentation for all existing tasks Aug 1, 2019
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI: auto-run Run CI automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants