Skip to content

Conversation

@WindQAQ
Copy link
Member

@WindQAQ WindQAQ commented Jun 15, 2019

  1. Do TODO : make tests compatible with graph mode in distort_image_ops.
  2. Revert Fix distort_image_ops_test #257.
  3. Have to use self.evaluate() to make exception raised in C++.

@WindQAQ WindQAQ requested a review from a team June 15, 2019 16:51
@WindQAQ WindQAQ added the image label Jun 15, 2019
@kyleabeauchamp
Copy link
Contributor

I think this all LGTM. The only question I have is whether we need to do anything more along the lines of to #260 / #290

@WindQAQ
Copy link
Member Author

WindQAQ commented Jun 15, 2019

Hi Kyle, initial codes migrated from contrib have only dynamic shape checking (check in graph building time). When decorating with tf.function, they randomly failed the CI testing (I don't know the reason...). As suggested in #260 (comment), doing static shape checking and dynamic shape checking seem to be the most robust and most correct way (see #260 (comment) for more details).

@facaiy
Copy link
Member

facaiy commented Jun 16, 2019

Hi, Tzu-Wei, what will happen without self.evaluate?

@WindQAQ
Copy link
Member Author

WindQAQ commented Jun 16, 2019

Hi, Facai, without self.evaluate, the test will fail in graph mode because the OpError is not raised.

@Squadrick
Copy link
Member

@WindQAQ Looks good, let's get this merged.

@Squadrick Squadrick merged commit cb09478 into tensorflow:master Jun 16, 2019
@WindQAQ WindQAQ deleted the make-tests-compatible-with-graph-mode branch June 16, 2019 17:29
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.

5 participants