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

Remove labels from audio #3513

Closed
wchargin opened this issue Apr 14, 2020 · 0 comments · Fixed by #3500
Closed

Remove labels from audio #3513

wchargin opened this issue Apr 14, 2020 · 0 comments · Fixed by #3500
Assignees

Comments

@wchargin
Copy link
Contributor

Most versions of the audio summary APIs only support writing audio data,
but the TB 1.x tensorboard.summary.audio ops also support attaching a
“label” (Markdown text) to each audio clip. This feature is not present
in any version of the TensorFlow summary APIs, including tf.contrib,
and is not present in the TB 2.x tensorboard.summary.audio API.

This feature was added in response to user request (#296), but we don’t
think that it has been widely adopted. The data can only be written by a
particular combination of writing APIs that is not commonly used, and
some standard Google-internal tools don’t support reading it.

The feature makes it harder to add generic data support to the audio
dashboard, so we’re removing it for now. If this proves unacceptable, we
can look into reinstating the functionality by a different mechanism,
likely a parallel tensor time series of labels only.

@wchargin wchargin self-assigned this Apr 14, 2020
wchargin added a commit that referenced this issue Apr 17, 2020
Summary:
Most versions of the audio summary APIs only support writing audio data,
but the TB 1.x `tensorboard.summary.audio` ops also support attaching a
“label” (Markdown text) to each audio clip. This feature is not present
in any version of the TensorFlow summary APIs, including `tf.contrib`,
and is not present in the TB 2.x `tensorboard.summary.audio` API. It
hasn’t been widely adopted, and doesn’t integrate nicely in its current
form with upcoming efforts like migrating the audio dashboard to use the
generic data APIs.

This commit causes labels to no longer be read by TensorBoard. Labels
are still written by the TB 1.x summary ops if specified, but now print
a warning. Because we do not change the write paths, it is important
that `audio.metadata` *not* set `DATA_CLASS_BLOB_SEQUENCE`, because
otherwise audio summaries will not be touched by `dataclass_compat`.
Tests for `dataclass_compat` cover this.

We don’t think that this feature has wide adoption. If this change gets
significant pushback, we can look into restoring labels with a different
implementation, likely as a parallel tensor time series.

Closes #3513.

Test Plan:
Unit tests updated. As a manual test, TensorBoard still works on both
legacy audio data and audio data in the new form (that’s not actually
written to disk yet), and simply does not display any labels.

wchargin-branch: audio-no-labels
caisq pushed a commit to caisq/tensorboard that referenced this issue May 19, 2020
Summary:
Most versions of the audio summary APIs only support writing audio data,
but the TB 1.x `tensorboard.summary.audio` ops also support attaching a
“label” (Markdown text) to each audio clip. This feature is not present
in any version of the TensorFlow summary APIs, including `tf.contrib`,
and is not present in the TB 2.x `tensorboard.summary.audio` API. It
hasn’t been widely adopted, and doesn’t integrate nicely in its current
form with upcoming efforts like migrating the audio dashboard to use the
generic data APIs.

This commit causes labels to no longer be read by TensorBoard. Labels
are still written by the TB 1.x summary ops if specified, but now print
a warning. Because we do not change the write paths, it is important
that `audio.metadata` *not* set `DATA_CLASS_BLOB_SEQUENCE`, because
otherwise audio summaries will not be touched by `dataclass_compat`.
Tests for `dataclass_compat` cover this.

We don’t think that this feature has wide adoption. If this change gets
significant pushback, we can look into restoring labels with a different
implementation, likely as a parallel tensor time series.

Closes tensorflow#3513.

Test Plan:
Unit tests updated. As a manual test, TensorBoard still works on both
legacy audio data and audio data in the new form (that’s not actually
written to disk yet), and simply does not display any labels.

wchargin-branch: audio-no-labels
caisq pushed a commit that referenced this issue May 27, 2020
Summary:
Most versions of the audio summary APIs only support writing audio data,
but the TB 1.x `tensorboard.summary.audio` ops also support attaching a
“label” (Markdown text) to each audio clip. This feature is not present
in any version of the TensorFlow summary APIs, including `tf.contrib`,
and is not present in the TB 2.x `tensorboard.summary.audio` API. It
hasn’t been widely adopted, and doesn’t integrate nicely in its current
form with upcoming efforts like migrating the audio dashboard to use the
generic data APIs.

This commit causes labels to no longer be read by TensorBoard. Labels
are still written by the TB 1.x summary ops if specified, but now print
a warning. Because we do not change the write paths, it is important
that `audio.metadata` *not* set `DATA_CLASS_BLOB_SEQUENCE`, because
otherwise audio summaries will not be touched by `dataclass_compat`.
Tests for `dataclass_compat` cover this.

We don’t think that this feature has wide adoption. If this change gets
significant pushback, we can look into restoring labels with a different
implementation, likely as a parallel tensor time series.

Closes #3513.

Test Plan:
Unit tests updated. As a manual test, TensorBoard still works on both
legacy audio data and audio data in the new form (that’s not actually
written to disk yet), and simply does not display any labels.

wchargin-branch: audio-no-labels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant