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

_ConstantValue can now see through tf.identity ops #38006

Merged
merged 3 commits into from Apr 28, 2020

Conversation

ngc92
Copy link
Contributor

@ngc92 ngc92 commented Mar 28, 2020

Fix for #37716

_ConstantValue (and thus get_static_value) now know that the static value of the result of tf.identity is the same as the static value of the original.

@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Mar 28, 2020
@gbaned gbaned self-assigned this Mar 29, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 29, 2020
@gbaned gbaned requested a review from alextp March 29, 2020 05:22
alextp
alextp previously approved these changes Mar 30, 2020
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Mar 30, 2020
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 30, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 30, 2020
@gbaned gbaned added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Apr 1, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 1, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Apr 9, 2020
@ngc92
Copy link
Contributor Author

ngc92 commented Apr 15, 2020

It seems the test failures are caused by tensorflow now being able to raise InvalidArgumentError in some more situations, which means that the confusion matrix tests that expect the op to assert fail with this exception. How should I proceed?

@gbaned
Copy link
Contributor

gbaned commented Apr 21, 2020

It seems the test failures are caused by tensorflow now being able to raise InvalidArgumentError in some more situations, which means that the confusion matrix tests that expect the op to assert fail with this exception. How should I proceed?

@alextp Can you please assist on above comments from @ngc92 . Thanks!

@gbaned gbaned removed the ready to pull PR ready for merge process label Apr 21, 2020
@alextp
Copy link
Contributor

alextp commented Apr 21, 2020

You can make changes to the tests to make them pass.

@gbaned
Copy link
Contributor

gbaned commented Apr 22, 2020

@ngc92 Can you please check @alextp comments and resolve conflicts?. Thanks!

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Apr 22, 2020
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Apr 22, 2020
@gbaned gbaned added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting response Status - Awaiting response from author labels Apr 22, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 22, 2020
alextp
alextp previously approved these changes Apr 22, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 22, 2020
…ntity

# Conflicts:
#	tensorflow/python/framework/tensor_util.py
#	tensorflow/python/kernel_tests/confusion_matrix_test.py
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Apr 22, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Apr 22, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 22, 2020
@gbaned gbaned removed the stat:awaiting response Status - Awaiting response from author label Apr 23, 2020
@ngc92 ngc92 requested a review from alextp April 23, 2020 17:09
@gbaned gbaned added the awaiting review Pull request awaiting review label Apr 24, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 24, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 24, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 24, 2020
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Apr 26, 2020
@gbaned gbaned added the kokoro:force-run Tests on submitted change label Apr 26, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 26, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Apr 28, 2020
@tensorflow-copybara tensorflow-copybara merged commit 5298742 into tensorflow:master Apr 28, 2020
@ngc92 ngc92 deleted the constant_from_identity branch May 3, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Approved by Reviewer
Development

Successfully merging this pull request may close these issues.

None yet

7 participants