hparams: write valid tensor summaries #3386
Merged
Conversation
This file contains hidden or 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
Summary: The summaries written by the hparams plugin only contain metadata, which is conceptually fine but not valid at an implementation level: all summary `Value`s must contain a `tensor`. Otherwise, the read paths get into confusing states where `SummaryMetadata` can be present for a tag but actually reading that tag’s `Tensors` raises `KeyError`. As of this patch, hparams summaries include a rank-1 length-0 float tensor. We also add a read-time `dataclass_compat` migration for legacy data. Test Plan: Calling `list_tensors` or `read_tensors` on a `MultiplexerDataProvider` now properly returns hparams data. Before this patch, it returned nothing as the summaries had no data class; if the null tensor backfill is removed from this patch, it fails with a `KeyError`. wchargin-branch: hparams-summary-tensors
wchargin-branch: hparams-summary-tensors wchargin-source: c18c31e59f0786f24887949068483618d21bfdc2
nfelt
approved these changes
Mar 17, 2020
|
|
||
| # Like `metadata.NULL_TENSOR`, but with the TensorFlow version of the | ||
| # proto. Slight kludge needed to expose the `TensorProto` type. | ||
| _TF_NULL_TENSOR = type(tf.compat.v1.Summary.Value().tensor).FromString( |
Contributor
There was a problem hiding this comment.
IMO a bit cleaner to do type(tf.make_tensor_proto(0)) for this?
wchargin-branch: hparams-summary-tensors wchargin-source: a4c4ac8c7ad15588e3fd5f202e196e6befeb9ba4
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary:
The summaries written by the hparams plugin only contain metadata, which
is conceptually fine but not valid at an implementation level: all
summary
Values must contain atensor. Otherwise, the read paths getinto confusing states where
SummaryMetadatacan be present for a tagbut actually reading that tag’s
TensorsraisesKeyError. As of thispatch, hparams summaries include a rank-1 length-0 float tensor. We also
add a read-time
dataclass_compatmigration for legacy data.Test Plan:
Calling
list_tensorsorread_tensorson aMultiplexerDataProvidernow properly returns hparams data. Before this patch, it returned
nothing as the summaries had no data class; if the null tensor backfill
is removed from this patch, it fails with a
KeyError.wchargin-branch: hparams-summary-tensors