Skip to content

Bug: Remove reciprocal from monotonic decreasing ops#25535

Merged
tensorflow-copybara merged 1 commit intotensorflow:masterfrom
lgeiger:inv-non-monotonic
Feb 9, 2019
Merged

Bug: Remove reciprocal from monotonic decreasing ops#25535
tensorflow-copybara merged 1 commit intotensorflow:masterfrom
lgeiger:inv-non-monotonic

Conversation

@lgeiger
Copy link
Copy Markdown
Contributor

@lgeiger lgeiger commented Feb 6, 2019

1 / x has an infinite discontinuity at x = 0 (https://www.wolframalpha.com/input/?i=1+%2F+x). Thus it is not monotonic on the entire real line.

The algorithmic optimizer would optimize the case Max(Inv(x)) => Inv(Min(x)). But this is only true for x > 0 or x < 0. E.g.:

x = tf.constant([-1.5, 1., 2.])
tf.reduce_max(tf.reciprocal(x))  # 1.0
tf.reciprocal(tf.reduce_min(x))  # -0.6666667

This PR removes Inv and Reciprocal ops from the list of elementwise monotonic ops.
Fixes #25330 (comment)

/cc @ezhulenev

@ezhulenev
Copy link
Copy Markdown
Contributor

Whoah, that's scary.

@rthadur rthadur self-assigned this Feb 6, 2019
@rthadur rthadur added awaiting review Pull request awaiting review kokoro:force-run Tests on submitted change ready to pull PR ready for merge process size:XS CL Change Size: Extra Small labels Feb 6, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 6, 2019
Copy link
Copy Markdown
Contributor

@rmlarsen rmlarsen left a comment

Choose a reason for hiding this comment

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

Thanks for catching this.

@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Feb 7, 2019
@tensorflow-copybara tensorflow-copybara merged commit f7fba1b into tensorflow:master Feb 9, 2019
tensorflow-copybara pushed a commit that referenced this pull request Feb 9, 2019
@lgeiger lgeiger deleted the inv-non-monotonic branch February 9, 2019 00:51
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

None yet

Development

Successfully merging this pull request may close these issues.

8 participants