Defer image/audio/histogram v2 summary preprocessing using LazyTensorCreator #2899
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
hongjunChoi
approved these changes
Nov 6, 2019
wchargin
approved these changes
Nov 7, 2019
| @@ -29,11 +29,14 @@ | |||
| from __future__ import division | |||
| from __future__ import print_function | |||
|
|
|||
| import functools | |||
|
|
||
| def test_reentrant_callable_does_not_deadlock(self): | ||
| @lazy_tensor_creator.LazyTensorCreator | ||
| def lazy_tensor(nested_call=False): |
There was a problem hiding this comment.
Is this nested_call=False vestigial?
There was a problem hiding this comment.
Yep, removed. Thanks for the catch.
yatbear
added a commit
to yatbear/tensorboard
that referenced
this pull request
Sep 30, 2021
To defer image/audio/histogram v2 summary preprocessing. See tensorflow#2899 for more details.
yatbear
added a commit
that referenced
this pull request
Oct 1, 2021
* use LazyTensorCreator To defer image/audio/histogram v2 summary preprocessing. See #2899 for more details. * add _buckets_v3 skeleton It's currently just a copy of _buckets(), modification for single value case handling will be added later.
yatbear
added a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
…low#5352) * use LazyTensorCreator To defer image/audio/histogram v2 summary preprocessing. See tensorflow#2899 for more details. * add _buckets_v3 skeleton It's currently just a copy of _buckets(), modification for single value case handling will be added later.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
…low#5352) * use LazyTensorCreator To defer image/audio/histogram v2 summary preprocessing. See tensorflow#2899 for more details. * add _buckets_v3 skeleton It's currently just a copy of _buckets(), modification for single value case handling will be added later.
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.
Continuation of PRs #2893, #2894, #2895 by @hongjunChoi .
This takes advantage of a recent change in
tf.summary.write()as of tensorflow/tensorflow@661f0ac that allows passing a callable as thetensorargument instead of an actualtf.Tensor. With this functionality,write()will call the callable to get the tensor only if the tensor is going to be used (i.e. the summary writer exists and the summary recording condition is met). This avoids overhead in executing preprocessing steps when they are not needed. Among V2 summaries, only image, audio, and histogram do any real preprocessing, so those are the only ones changed in this PR.In order to maintain backwards compatibility with the previous
write()API, this introduces a shim layer calledLazyTensorCreatorthat usestf.register_tensor_conversion_function()to ensure that the callable we pass in can be automatically converted to atf.Tensorin many contexts (and in particular, in the context of the call totf.identity()which is used insidewrite()). That way a version skew (e.g. TensorBoard 2.1 but TensorFlow 2.0) won't break the summary API integration; while technically we don't guarantee that a skew in this direction will work, this is a likely enough situation and a confusing enough error that it's worth a bit of work to avoid.cc @alextp FYI