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

[Object_Detection] Add IOUSimilarity layer. #3

Closed
wants to merge 2 commits into from
Closed

Conversation

tanzhenyu
Copy link
Contributor

Todos with ragged tensor support.

@tanzhenyu tanzhenyu requested a review from fchollet June 16, 2020 17:41
@bhack
Copy link
Contributor

bhack commented Jun 16, 2020

@tanzhenyu
Copy link
Contributor Author

https://github.com/tensorflow/addons/blob/v0.10.0/tensorflow_addons/losses/giou_loss.py#L25-L61

Making it used for loss seems pretty easy -- just inherit from LossFunctionWrapper instead of Base Layer? I'm fine either way, depending on whether many people use iou directly as loss or not.

@bhack
Copy link
Contributor

bhack commented Jun 16, 2020

Making it used for loss seems pretty easy -- just inherit from LossFunctionWrapper instead of Base Layer? I'm fine either way, depending on whether many people use iou directly as loss or not.

@gabrieldemarmiesse what do you think?

@gabrieldemarmiesse
Copy link

I believe both have their use cases and it would be nice to have both. But this isn't blocking this pull request in any way. No need to add everything at once. We can always put it in the backlog.

@bhack
Copy link
Contributor

bhack commented Jun 16, 2020

@gabrieldemarmiesse Ok thanks in the meantime we could use the Layer from keras-cv and the loss from Tensorflow-addons.

@tanzhenyu
Copy link
Contributor Author

@gabrieldemarmiesse Ok thanks in the meantime we could use the Layer from keras-cv and the loss from Tensorflow-addons.

@bhack @gabrieldemarmiesse Feel free to add comments & feedbacks or tag others that you think might be interested in this, always good to improve from the very beginning :-)

@bhack
Copy link
Contributor

bhack commented Jun 16, 2020

In addons was introduced as loss in 2019 by @fsx950223 with tensorflow/addons#477

import tensorflow as tf


class IOUSimilarity(tf.keras.layers.Layer):
Copy link
Contributor

@fchollet fchollet Jun 16, 2020

Choose a reason for hiding this comment

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

Big question here is why a layer -- what workflows will it be used in? We do already have a IoU metric -- would it make sense for IoU to be a metric and a loss instead of a layer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The more I think about it, this is actually pretty different from what 1) giou loss in addons, 2) iou in keras/metrics is doing.
Both 1) and 2) require y_true and y_pred, which needs to be the same shape, i.e.,:
y_pred: [n_boxes, 4]
y_true: [n_boxes, 4]
And the metrics/loss from it is [n_boxes] before reduction

However what we are having here is:
box1: [n_boxes_1, 4]
box2: [n_boxes_2, 4]
and we're computing the bipartite similarities, so the result would be [n_boxes_1, n_boxes_2]

More specifically, we will need to call this layer with call(gt_boxes, anchors), where n_boxes_1 is usually 3-10 (which is also why I added ragged support because we don't know how many), but n_boxes_2 is always fixed (e.g., 8732 in SSD300).
This is really used for different things, so I believe it should be a layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect the output of this layer to be fed to other layers? Or is it a final output of the model? If the latter, then maybe it should be a metric instead (to be used instead another layer, for instance).

Choose a reason for hiding this comment

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

IMO, this should be a metric

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that IOU and derivates could be still interesting also as loss:
https://arxiv.org/abs/1908.03851
https://arxiv.org/abs/1911.08287

@tanzhenyu tanzhenyu closed this Sep 12, 2020
@tanzhenyu tanzhenyu deleted the iou branch September 17, 2020 22:43
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Oct 30, 2022
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Nov 3, 2022
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Jan 20, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Jan 29, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Feb 8, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Mar 22, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Mar 22, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Mar 27, 2023
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Mar 29, 2023
LukeWood pushed a commit that referenced this pull request Apr 11, 2023
…d update iou losses (#1296)

* first attempt at introducing YoloX

* formatted and fixed bugs

* cast fix #1

* cast fix #2

* cast fix #3

* cast fix #4

* adding ensure shape for support

* reverting and removing ensure_shape

* fixed another bug

* updated train.py

* updated docs, tests and added support for loss strings

* first attempt at introducing YoloX

* formatted and fixed bugs

* adding ensure shape for support

* reverting and removing ensure_shape

* reformatted by black

* fixed a  linting issue

* finally rebased atop the recent changes

* finally rebased atop the new changes

* fixed linting issues

* reverted rebasing issues with iou loss

* fixing rebased errors part 2

* fixed more linting issues

* TPU testing changes

* linting fixes

* updated with implementation details from paper

* updated based on review comments and api changes

* first attempt at introducing YoloX

* updated docs, tests and added support for loss strings

* fixed linting issues

* reverted rebasing issues with iou loss

* review comments

* removed examples

* linting fix

* fixed rebasing error

* updated no_reduction warning

* review comments

* revert version and linting fixes
quantumalaviya added a commit to quantumalaviya/keras-cv that referenced this pull request Apr 24, 2023
freedomtan pushed a commit to freedomtan/keras-cv that referenced this pull request Jul 20, 2023
* Add numpy ops (initial batch) and some config

* Add unit test

* fix call

* Revert "fix call"

This reverts commit 6748ad183029ff4b97317b77ceed8661916bb9a0.

* full unit test coverage

* fix setup.py

* more ops and fix the problem of KerasTensor [ops] numerics
ghost pushed a commit to y-vectorfield/keras-cv that referenced this pull request Nov 16, 2023
…d update iou losses (keras-team#1296)

* first attempt at introducing YoloX

* formatted and fixed bugs

* cast fix keras-team#1

* cast fix keras-team#2

* cast fix keras-team#3

* cast fix keras-team#4

* adding ensure shape for support

* reverting and removing ensure_shape

* fixed another bug

* updated train.py

* updated docs, tests and added support for loss strings

* first attempt at introducing YoloX

* formatted and fixed bugs

* adding ensure shape for support

* reverting and removing ensure_shape

* reformatted by black

* fixed a  linting issue

* finally rebased atop the recent changes

* finally rebased atop the new changes

* fixed linting issues

* reverted rebasing issues with iou loss

* fixing rebased errors part 2

* fixed more linting issues

* TPU testing changes

* linting fixes

* updated with implementation details from paper

* updated based on review comments and api changes

* first attempt at introducing YoloX

* updated docs, tests and added support for loss strings

* fixed linting issues

* reverted rebasing issues with iou loss

* review comments

* removed examples

* linting fix

* fixed rebasing error

* updated no_reduction warning

* review comments

* revert version and linting fixes
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.

5 participants