Identity box coder, similarity calculator, target assigner #8962

kmindspark · 2020-07-24T20:46:36Z

Description

📝 Please include a summary of the change.

Please also include relevant motivation and context.

List any dependencies that are required for this change.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
Documentation update
TensorFlow 2 migration
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
A new research paper code implementation
Other (Specify)

Tests

📝 Please describe the tests that you ran to verify your changes.

Provide instructions so we can reproduce.

Please also list any relevant details for your test configuration.

Test Configuration:

Checklist

I have signed the Contributor License Agreement.
I have read guidelines for pull request.
My code follows the coding guidelines.
I have performed a self code review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.

bhack · 2020-07-28T21:59:09Z

research/object_detection/core/box_list_ops.py

+        tf.expand_dims(tf.transpose(w1), axis=1))
+    return ycenters + xcenters + heights + widths
+
+def giou_loss(boxlist1, boxlist2, scope=None):


It is already available at https://www.tensorflow.org/addons/api_docs/python/tfa/losses/GIoULoss
If you need any extension please open a PR in TFA. /cc @tensorflow/sig-addons-maintainers

@tombstone told me to implement this separately so we wouldn't need that dependency.

Ok thanks.
@tombstone Our check it is just part of the approved RFC at https://github.com/tensorflow/community/blame/master/rfcs/20190802-model-garden-redesign.md#L60-L61

Another GIOU Is landing from #9083

tombstone

Thanks @kmindspark

tombstone · 2020-08-10T22:24:30Z

research/object_detection/core/region_similarity_calculator.py

@@ -31,11 +31,11 @@
 from object_detection.core import box_list_ops
 from object_detection.core import standard_fields as fields

-


Internal lint required two lines.

tombstone · 2020-08-10T22:33:51Z

research/object_detection/box_coders/detr_box_coder.py

+EPSILON = 1e-8
+
+
+class DETRBoxCoder(box_coder.BoxCoder):


As discussed offline, it might be best to follow CenterNet's pattern here to create a single class that does the target assignment.

tombstone

Thanks @kmindspark. I left some comments. PTAL

tombstone · 2020-08-11T05:27:36Z

research/object_detection/core/region_similarity_calculator.py

@@ -35,7 +35,8 @@
 class RegionSimilarityCalculator(six.with_metaclass(ABCMeta, object)):
  """Abstract base class for region similarity calculator."""

-  def compare(self, boxlist1, boxlist2, scope=None):
+  def compare(self, boxlist1, boxlist2, scope=None,
+              groundtruth_labels=None, predicted_labels=None):


groundtruth_labels and predicted_labels can be fields inside boxlist. We don't need to add additional arguments to the interface.

tombstone · 2020-08-11T05:32:51Z

research/object_detection/core/region_similarity_calculator.py

+    """
+    classification_scores = tf.matmul(groundtruth_labels,
+        tf.nn.softmax(predicted_labels), transpose_b=True)
+    return -5 * box_list_ops.l1(boxlist1, boxlist2) + 2 * box_list_ops.giou(


add the scale factors 5 and 2 as constructor arguments. Also are these values mentioned in the paper ?

tombstone · 2020-08-11T05:37:08Z

research/object_detection/core/region_similarity_calculator.py

+    """
+    classification_scores = tf.matmul(groundtruth_labels,
+        tf.nn.softmax(predicted_labels), transpose_b=True)
+    return -5 * box_list_ops.l1(boxlist1, boxlist2) + 2 * box_list_ops.giou(


shouldn't the second term be -2 * box_list_ops.giou(..) ?

It is + because this is calculating similarity, and box list ops giou is not doing 1-giou

tombstone · 2020-08-11T05:41:48Z

research/object_detection/core/target_assigner.py

+  """Target assigner to compute classification and regression targets."""
+
+  def __init__(self,
+               similarity_calc,


Let's hardcode this class to use DETRSimilarityCalc

tombstone · 2020-08-11T05:43:09Z

research/object_detection/core/target_assigner.py

+    self._negative_class_weight = negative_class_weight
+
+  def assign(self,
+             anchors,


why do we have anchors here ? Do you mean predicted boxes ?

tombstone · 2020-08-11T05:46:09Z

research/object_detection/core/target_assigner.py

+             anchors,
+             groundtruth_boxes,
+             groundtruth_labels=None,
+             unmatched_class_label=None,


I don't think we need unmatched_class_label for DETR.

Why is that? Should I hard code it to [1] + [0]*num_classes inside target assigner?

Needed to keep compatible with batch_assign_targets so decided to leave this in, is that ok?

tombstone · 2020-08-11T05:46:53Z

research/object_detection/core/target_assigner.py

+        representing weights for each element in cls_targets.
+      reg_targets: a float32 tensor with shape [num_anchors, box_code_dimension]
+      reg_weights: a float32 tensor with shape [num_anchors]
+      match: an int32 tensor of shape [num_anchors] containing result of anchor


no need to return match object. Let's simplify.

In that case it would be difficult to use the existing batch_assign_targets right? Which would be nice to use.

tombstone · 2020-08-11T05:47:46Z

research/object_detection/core/target_assigner.py

+                                                  0))
+      groundtruth_labels = tf.expand_dims(groundtruth_labels, -1)
+
+    unmatched_shape_assert = shape_utils.assert_shape_equal(


remove shape asserts.

tombstone · 2020-08-11T05:49:08Z

research/object_detection/core/target_assigner.py

+
+  def __init__(self,
+               similarity_calc,
+               matcher,


hardcode to use hungarian matcher.

tombstone · 2020-08-11T05:50:27Z

research/object_detection/core/target_assigner.py

+      cls_weights = tf.tile(cls_weights, weights_multiple)
+
+    num_anchors = anchors.num_boxes_static()
+    if num_anchors is not None:


can you rename anchors throughout to be predicted_boxes ?

tombstone

Thanks @kmindspark

tombstone · 2020-08-14T06:26:03Z

research/object_detection/core/target_assigner.py

@@ -107,7 +108,8 @@ def assign(self,
             groundtruth_boxes,
             groundtruth_labels=None,
             unmatched_class_label=None,
-             groundtruth_weights=None):
+             groundtruth_weights=None,
+             class_predictions=None):


You don't need this change

I do because in batch_assign it by default now passes in class predictions.

tombstone · 2020-08-14T06:27:09Z

research/object_detection/core/target_assigner.py

@@ -434,6 +436,9 @@ def create_target_assigner(reference, stage=None,
                                           use_matmul_gather=use_matmul_gather)
    box_coder_instance = faster_rcnn_box_coder.FasterRcnnBoxCoder()

+  elif reference == 'DETR':


we don't need to add it here. In DETR meta architecture you can directly create DETRTargetAssigner()

tombstone · 2020-08-14T06:36:45Z

research/object_detection/core/target_assigner.py

@@ -1897,3 +1911,197 @@ def assign_corner_offset_targets(

    return (tf.stack(corner_targets, axis=0),
            tf.stack(foreground_targets, axis=0))
+
+class DETRTargetAssigner(object):


I am still a bit worried that we are carrying around a lot of legacy code for the DETR Target Assigner. Can you take a look at https://github.com/facebookresearch/detr/blob/master/models/matcher.py and see if we can do something similiar ?

That file just does the matching. Do you mean the fact that we do similarity calculation separately from the target assigner?

tombstone

Thanks @kmindspark . This is looking good.

research/object_detection/core/region_similarity_calculator.py

research/object_detection/core/target_assigner.py

research/object_detection/core/target_assigner_test.py

tombstone · 2020-08-18T17:44:23Z

research/object_detection/core/target_assigner.py

+        groundtruth_boxes,
+        box_preds)
+    match = self._matcher.match(match_quality_matrix,
+                                valid_rows=tf.greater(groundtruth_weights, 0))


question - where do we ensure that the number of predicted boxes and number of padded groundtruth boxes is equal ?

the default number of boxes to pad to in the config (max_number_of_boxes) is 100. I could explicitly set this in the config. I can also add an assert to confirm.

tombstone · 2020-08-18T17:45:19Z

research/object_detection/core/target_assigner.py

+        groundtruth_boxes,
+        box_preds)
+    match = self._matcher.match(match_quality_matrix,
+                                valid_rows=tf.greater(groundtruth_weights, 0))


In practice does DETR use zeros to represent null groundtruths ?

That's how the data loading works I believe (giving 0 weights to padded boxes), if that's not the case let me know 👍

tombstone · 2020-08-26T15:19:58Z

research/object_detection/detr-gpu-pets.config

@@ -0,0 +1,118 @@
+# Faster R-CNN with Resnet-50 (v1) with 640x640 input resolution


remove this config from the PR

oops not sure how this snuck in.

tombstone

Thanks @kmindspark

kmindspark added 3 commits July 24, 2020 19:09

target assigner mods

0006ba7

progress on pr

c99e578

pr

c594cec

kmindspark requested review from jch1, pkulzc and tombstone as code owners July 24, 2020 20:46

googlebot added the cla: yes label Jul 24, 2020

kmindspark added 10 commits July 24, 2020 20:48

Merge remote-tracking branch 'upstream/master' into detr-push-3

7359586

save progress

9bd3fe6

ready pr

ceb406b

updates

c95500c

Merge remote-tracking branch 'upstream/master' into detr-push-3

4753d6e

fix

55db4ad

fix

70cb851

minor style elements

322d444

fix tests

ba65cc7

fix tests

7b165eb

bhack reviewed Jul 28, 2020

View reviewed changes

This was referenced Jul 28, 2020

Ask contribution to Tensorflow addons for general scope utils, loss, layers, ops tensorflow/community#223

Closed

[Help wanted] DETR (DEtection TRansformer): End-to-End Object Detection with Transformers #8707

Closed

kmindspark added 10 commits July 29, 2020 06:04

fix

3564e7c

fix

a679728

fix file

8948ba3

fix

5f71a45

fix pr

7723b20

fix

980d176

fix

dabfc27

fix this pr

f0bc684

self code-review to clean up

4022aae

self code-review to clean up

22b5b0c

kmindspark added 2 commits August 7, 2020 22:33

remove detr lib

4f7965f

fix documentation for box coder

e0b082e

tombstone suggested changes Aug 10, 2020

View reviewed changes

kmindspark added 4 commits August 11, 2020 00:35

separate out DETR

ab96cb3

remove box coders

8e77b75

add line

3d757d5

compress target assigner

4f135c7

tombstone suggested changes Aug 11, 2020

View reviewed changes

kmindspark added 2 commits August 11, 2020 19:01

make suggested fixes to target assigner and similarity calculator

d54c86d

target assigner and similarity calculator fixes

e09e056

tombstone suggested changes Aug 14, 2020

View reviewed changes

jaeyounkim assigned jch1, tombstone and pkulzc Aug 14, 2020

kmindspark added 4 commits August 15, 2020 05:02

fixes to simplify

8f5ed2d

fixes to simplify

656ec2a

clean target assigner

9d4b102

remove detr building

323ea89

tombstone suggested changes Aug 18, 2020

View reviewed changes

kmindspark added 5 commits August 18, 2020 18:38

work on cleaning up further

a6f36d2

target assigner test

111c9d3

fix issues with target assigner for pr

de3a34b

fix num classes

e350c59

fix naming

1ed7ef3

tombstone reviewed Aug 26, 2020

View reviewed changes

flip around similarity calculation

0bc599e

tombstone approved these changes Aug 27, 2020

View reviewed changes

tombstone added the ready to pull ready to pull (create internal pr review and merge automatically) label Aug 27, 2020

tf-models-copybara-bot merged commit 13a030e into tensorflow:master Sep 15, 2020

bhack mentioned this pull request Jan 28, 2022

Initial commit for the keras-cv roadmap/community guide line keras-team/keras-cv#61

Merged

		@@ -31,11 +31,11 @@
		from object_detection.core import box_list_ops
		from object_detection.core import standard_fields as fields

		@@ -0,0 +1,118 @@
		# Faster R-CNN with Resnet-50 (v1) with 640x640 input resolution

Identity box coder, similarity calculator, target assigner #8962

Identity box coder, similarity calculator, target assigner #8962

Conversation

kmindspark commented Jul 24, 2020

Description

Type of change

Tests

Checklist

bhack Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmindspark Aug 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombstone left a comment

Choose a reason for hiding this comment

bhack Jul 28, 2020 •

edited

Loading

kmindspark Aug 11, 2020 •

edited

Loading