Skip to content

Conversation

@derekjchow
Copy link
Contributor

No description provided.

Bug:
We use two different Saver objects during training, one to initialize from a
checkpoint and another to save and restore from checkpoints during training.
Both these Savers were bound to same variable name causing init_fn function
to use the latter instead of the former when called.
This change is required to enable object detection export and inference
with dynamic batch size.
A few change to prepare for batch inference:

* Modify the return type of batch non max suppression to be tuple of tensors
  so it can be reused for both stages of faster rcnn without any confusion
  in the semantics implied the the keys used to represent the tensors.
* Allow dynamic number of anchors (boxes) in addition to dynamic batch size.
* Remove a redundant dynamic batch size test.
* Adds a util function to compute a mix of dynamic and static shapes
  preferring static when available.
* Uses batch_multiclass_non_max_suppression function in postprocess_rpn
  instead of looping over static batch shape and performing
  multiclass_non_max_suppression.
* Adds a new helper function _unpad_proposals_and_sample_boxclassifier_batch
  to sample from a batch of tensors possibly containing paddings.
* Tests batch inference with various configurations of static shape via
  unittests.
* Creates a new batch_decode method in SSD Meta architecture that can handle
  dynamic batch size.
* use combined_shapes in _get_feature_maps_spatial_dims method to handle
  dynamic batch image_size.
* Add dynamic batch size tests to check preprocess, predict and postprocess
  methods in SSD Meta architecture.
TODO: revisit whether it's possible to force the `Repeat` namescope as
created in `_extract_box_classifier_features` to start counting at 2 (e.g.
`Repeat_2`) so that the default restore_fn can be used.
TODO(jonathanhuang,rathodv): revisit whether it's possible to force the
Copy link
Contributor

Choose a reason for hiding this comment

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

can you purge the names ?

Copy link
Contributor

@tombstone tombstone left a comment

Choose a reason for hiding this comment

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

Looks good Derek. Just one nit. Thanks for merging this humungous set of changes !

@derekjchow
Copy link
Contributor Author

PTAL.

Copy link
Contributor

@tombstone tombstone left a comment

Choose a reason for hiding this comment

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

Looks good to Me. Please run affected tests and a few steps of train and eval before merging. Thanks!

@derekjchow derekjchow merged commit 582bf92 into tensorflow:master Jul 28, 2017
@jlertle
Copy link

jlertle commented Jul 29, 2017

@derekjchow you rock!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants