Skip to content

Blend Image Op #1275

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

Merged
merged 12 commits into from
Mar 19, 2020
Merged

Blend Image Op #1275

merged 12 commits into from
Mar 19, 2020

Conversation

abhichou4
Copy link
Contributor

@abhichou4 abhichou4 commented Mar 9, 2020

Refers #1333 (#1226)
Adding blend op to tensorflow_addons.image

@seanpmorgan
Copy link
Member

#1226
For now, I've just added image ops. Here's a notebook to make it easy to test them. I'm working on adding tests for AutoAugment, and adding Hparams for training from the tensor2tensor repo too. Please let me know if this is what's expected, and how this migration should be broken down into PRs.

Hi @abhichou4 thanks for starting this! So when we moved from tf.contrib we decided not to bring over HParams since it's a new API and that was against our charter. I'm wondering if we can utilize KerasTuner's Hyperparameter object since this will be maintained going forward without our help:
https://github.com/keras-team/keras-tuner/blob/master/kerastuner/engine/hyperparameters.py#L464

@lgeiger
Copy link
Contributor

lgeiger commented Mar 10, 2020

Thanks for getting this started 👍

I'm wondering if we can utilize KerasTuner's Hyperparameter object since this will be maintained going forward without our help

Since HParams are only used in very few places here, I think a normal dict or a small dataclass should do the job here, without needing an extra dependency or additional maintenance overhead.

@bhack
Copy link
Contributor

bhack commented Mar 11, 2020

Hi @abhichou4 thanks for starting this! So when we moved from tf.contrib we decided not to bring over HParams since it's a new API and that was against our charter. I'm wondering if we can utilize KerasTuner's Hyperparameter object since this will be maintained going forward without our help:
https://github.com/keras-team/keras-tuner/blob/master/kerastuner/engine/hyperparameters.py#L4

/cc @omalleyt12

Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

I'm happy to see this going forward. Small pull requests are the best :)
A few comments to adress and we're good to merge.

Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

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

I think to fix the formatting errors you should run bash tools/pre-commit.sh. If you encounter any issues with it, please let me know.

@gabrieldemarmiesse gabrieldemarmiesse self-assigned this Mar 16, 2020
@abhichou4 abhichou4 changed the title AutoAugment and RandAugment Image Ops Blend Image Op Mar 18, 2020
@gabrieldemarmiesse
Copy link
Member

@abhichou4 Thanks for fixing the build. Could you keep the tests that you made before too? We never have too many tests.

@gabrieldemarmiesse
Copy link
Member

@abhichou4 could you remove the test on the image in the git repo? Afterwards we're good to merge this.

@gabrieldemarmiesse
Copy link
Member

Thanks again for the pull request!

@gabrieldemarmiesse gabrieldemarmiesse merged commit 2595947 into tensorflow:master Mar 19, 2020
jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
* autoaugment, no changes

* autoaugment with tfa and tf2

* removed contrib_training, hparams for now

* Delete hparam.py

* init augment_ops, added blend

* typo in BUILD

* added random test, round, unint8 ValueError

* fixed build

* image test

* add previous tests

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

Successfully merging this pull request may close these issues.

7 participants