Make pr_curves_demo manually implement clip #1132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TensorFlow commits tensorflow/tensorflow@083cf6b and tensorflow/tensorflow@daf0b20 have made their way into `tf-nightly` build 254. The commits modified the behavior of the `tf.clip_by_value` so that they ever so slightly vary based on system characteristics in an attempt I believe to minimize memory usage. Unfortunately, our python 2 and python 3 setups on travis differ along those characteristics, causing the `tf-nightly` build to break `:pr_curves_test` (which relies on the demo to generate test data) for python 2 only. This PR fixes the test by removing usages of `tf.clip_by_value` from the demo and instead uses `tf.maximum(minValue, tf.minimum(maxValue, value))`. The most fulfilling solution for this bug would be to resolve the mismatch in behavior of `tf.clip_by_value` on the TensorFlow side. However, this PR unblocks development of TensorBoard immediately.
jart
approved these changes
Apr 15, 2018
chihuahua
added a commit
to chihuahua/tensorboard
that referenced
this pull request
Apr 23, 2018
Previously, tensorflow#1132 had circumvented the use of `tf.clip_by_value` within the PR curves demo because it was buggy. However, that behavior seems to have been fixed.
nfelt
pushed a commit
to nfelt/tensorboard
that referenced
this pull request
Apr 24, 2018
TensorFlow commits tensorflow/tensorflow@083cf6b and tensorflow/tensorflow@daf0b20 have made their way into `tf-nightly` build 254. The commits modified the behavior of the `tf.clip_by_value` op in an attempt to resolve tensorflow/tensorflow#7225 in a way that minimizes memory usage by taking different pathways of logic based on characteristics of the system running TensorFlow. Unfortunately, our python 2 and python 3 setups on travis differ along those characteristics, causing the `tf-nightly` build to break `:pr_curves_test` (which relies on the demo to generate test data) for python 2 only. This PR fixes the test by removing usages of `tf.clip_by_value` from the demo and instead uses `tf.maximum(minValue, tf.minimum(maxValue, value))`. The most fulfilling solution for this bug would be to resolve the mismatch in behavior of `tf.clip_by_value` on the TensorFlow side. However, this PR unblocks development of TensorBoard immediately.
nfelt
pushed a commit
that referenced
this pull request
Apr 24, 2018
TensorFlow commits tensorflow/tensorflow@083cf6b and tensorflow/tensorflow@daf0b20 have made their way into `tf-nightly` build 254. The commits modified the behavior of the `tf.clip_by_value` op in an attempt to resolve tensorflow/tensorflow#7225 in a way that minimizes memory usage by taking different pathways of logic based on characteristics of the system running TensorFlow. Unfortunately, our python 2 and python 3 setups on travis differ along those characteristics, causing the `tf-nightly` build to break `:pr_curves_test` (which relies on the demo to generate test data) for python 2 only. This PR fixes the test by removing usages of `tf.clip_by_value` from the demo and instead uses `tf.maximum(minValue, tf.minimum(maxValue, value))`. The most fulfilling solution for this bug would be to resolve the mismatch in behavior of `tf.clip_by_value` on the TensorFlow side. However, this PR unblocks development of TensorBoard immediately.
nfelt
pushed a commit
that referenced
this pull request
Apr 26, 2018
Previously, #1132 had circumvented the use of `tf.clip_by_value` within the PR curves demo because it was buggy. However, that behavior seems to have been fixed.
nfelt
added a commit
that referenced
this pull request
Apr 27, 2018
This addresses the root cause of the issue detected and originally addressed in #1132, per my description in #1150 (review).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TensorFlow commits tensorflow/tensorflow@083cf6b and tensorflow/tensorflow@daf0b20 have made their way into
tf-nightlybuild 254. The commits modified the behavior of thetf.clip_by_valueop in an attempt to resolve tensorflow/tensorflow#7225 in a way that minimizes memory usage by taking different pathways of logic based on characteristics of the system running TensorFlow. Unfortunately, our python 2 and python 3 setups on travis differ along those characteristics, causing thetf-nightlybuild to break:pr_curves_test(which relies on the demo to generate test data) for python 2 only.This PR fixes the test by removing usages of
tf.clip_by_valuefrom the demo and instead usestf.maximum(minValue, tf.minimum(maxValue, value)). The most fulfilling solution for this bug would be to resolve the mismatch in behavior oftf.clip_by_valueon the TensorFlow side. However, this PR unblocks development of TensorBoard immediately.