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.

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

Added augmentations #749

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

Om-Doiphode
Copy link
Contributor

Fixes #735

  • Added support for RandomSizedBBoxSafeCrop and PadIfNeeded in the deepforest/augmentations.py file
  • Added a doc page for augmentations
  • Modified the deepforest_config.yml file to include default and user defined augmentations

Copy link
Member

@ethanwhite ethanwhite left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @Om-Doiphode! It's definitely going to be super useful having easier flexible augmentation for training.

I have a couple of interrelated questions about design that I'd like us to think through on this one:

  1. Should the defaults be included in the config file or in the code?
  2. If they are stored in the config should it be a separate default_augmentation section or just be what is provided under the augmentations section in the default config file?
  3. Should the augmentation config be stored in our config file, or should it be stored in a separate config file following the albumentations JSON format?

I'll preface my thoughts on these by acknowledging that I'm not familiar with how this is normally handled so I'm happy to defer to others or look at examples from other packages.

1. If we have defaults that are basically always applied in our current code and we think we are almost always going to want to apply them I lean towards keeping those in the code. However if we envision users wanted to regularly change those defaults then I think it's reasonable to include them in a config file.

2. Typically in this sort of case the default values would just be what is in the relevant section of the default config file. So instead of having a default_augmentations section, those augmentations would be what is in augmentations section. We would then provide instructions for how to modify this section. One way that this is done (in addition to docs) is by having chunks of the config file commented out with a header line that says something like "Uncomment the code below if you want to add the PadIfNeed augmentation".

3. albumentations already has a loadable config file format that can be stored as either YAML or JSON. The YAML version looks quite similar to the YAML design in this PR. The advantage of using this format and moving to a separate augmentation config file is that it eliminates a lot of new custom code for reading and setting up augmentations.

So, having written all of this out my initial thought is that we should use the existing albumentations config and associate loading code and either have our defaults in the default albumentations config file provided or have them created in the code and then automatically combined with the additional augmentations provided in the albumentations config file. What do you think @bw4sz & @henrykironde?

@bw4sz
Copy link
Collaborator

bw4sz commented Aug 15, 2024

I'm back monday, this will have to wait till then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved zoom augmentations through albumentations.
3 participants