audio: deprecate labels and stop reading them #3500

wchargin · 2020-04-09T18:19:14Z

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

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. 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

nfelt

I have two slight quibbles with the PR name:

It doesn't really capture that this also changes dataclass_compat.py (which is significant for places that would use it outside the multiplexer, like logdir loader)
We aren't actually removing the label-displaying logic from the frontend; all this does is it stops passing the strings to the frontend. We should probably have a cleanup issue to track doing the first part as well (as commented separately).

Could be addressed by changing title to audio: deprecate labels and stop reading them or something similar.

nfelt · 2020-04-09T19:18:09Z

tensorboard/plugins/audio/summary.py



+_LABELS_WARNING = (
+    "Labels on audio summaries are deprecated and will be removed."


Maybe we should file an issue for this (to track the actual ultimate removal, including the actual UI elements) and include that here as a reference in case anyone notices and wants to object?

Done: #3513.

Summary: We initially used `dataclass_compat` to perform filtering of large graphs as a stopgap mechanism. This commit moves that filtering into the uploader, which is the only surface in which it’s actually used. As a result, `dataclass_compat` no longer takes extra arguments and so can be moved into `EventFileLoader` in a future change. Test Plan: Unit tests added to the uploader for the small graph, large graph, and corrupt graph cases. wchargin-branch: uploader-graph-filtering wchargin-source: 00af6ebe200fe60aacbc23f468eb44126d2d33f7

Summary: The `data_compat` and `dataclass_compat` transforms are now effected by the `EventFileLoader` class. The event accumulator and uploader therefore no longer need to effect these transformations manually. Test Plan: Unit tests that use mocking have been updated to apply compat transforms manually. Using the uploader with `--plugin scalars,graphs` still works as intended. wchargin-branch: efl-compat-transforms wchargin-source: d5b86d851f5dff6f24ddc8cfac52c740ab8af1b3

Summary: The `dataclass_compat` transform dispatches on plugin name, which is stored in the summary metadata. But the semantics of summary metadata is that it need only be set in the first event of each time series. Thus, until now, events past the first step of a time series have not been modified by `dataclass_compat`. This has been fine because all transformations have been either (a) only metadata transforms (which don’t matter after the first step anyway) or (b) tensor transforms to hparams data (which only ever has one step anyway). But an upcoming change will need to transform every step of each audio time series, so we need to identify which summaries have audio data even when they don’t have plugin name set on the same event. To do so, we let `dataclass_compat` update a stateful map from tag name to the first `SummaryMetadata` seen for that time series (scoped to a particular run). Test Plan: Some unit tests included now; the tests for migration of audio summaries will be more complete because they have more interesting things to test. wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 364d3b33f32452e8d3ac9468e6bcca4de876963a

wchargin-branch: uploader-graph-filtering wchargin-source: 9179acc76158380d9f254df1d0a0aae1ec82df6c

wchargin-branch: efl-compat-transforms wchargin-source: 4fa593a34147c7b05c7f4c55f3ce9279242a7414

wchargin-branch: audio-no-labels wchargin-source: 7c7844bff26668d32d692ef7faa0dba3ecbaf138 # Conflicts: # tensorboard/dataclass_compat.py

wchargin-branch: audio-no-labels wchargin-source: 7c7844bff26668d32d692ef7faa0dba3ecbaf138

wchargin

I have two slight quibbles with the PR name: […]

Fair points; done, with your wording.

wchargin · 2020-04-14T03:27:47Z

tensorboard/plugins/audio/summary.py



+_LABELS_WARNING = (
+    "Labels on audio summaries are deprecated and will be removed."


Done: #3513.

wchargin-branch: uploader-graph-filtering wchargin-source: b89f6da9cd740080aefecec49194752b0de85dd4

wchargin-branch: efl-compat-transforms wchargin-source: 8abfd3ec5c7394e7b1930b4bf42d2de816e67da4 # Conflicts: # tensorboard/uploader/uploader.py

wchargin-branch: efl-compat-transforms wchargin-source: 8abfd3ec5c7394e7b1930b4bf42d2de816e67da4

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: cfd81441162803b6bab104bc76d93c3b00e8c381

wchargin-branch: audio-no-labels wchargin-source: eb55e6cf2dcb2dc50a10201296e04c3a030bca3d

Summary: Fixes an oversight in #3507: we can’t assert that the raw bytes are what was expected because the code under test does a proto serialization roundtrip, which is permitted to permute keys. Test Plan: Tests still pass, and this should fix an internal sync error. wchargin-branch: uploader-test-proto-equal

wchargin-branch: uploader-graph-filtering wchargin-source: 30f0764698d66cdaf481c797dc56bf6a34ae3ae0

wchargin-branch: efl-compat-transforms wchargin-source: f18ea5648a96010b48237fe7743e5f5d0d535acb

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 9f516c3b25569b4e007bb929bb456541e5433f52

wchargin-branch: audio-no-labels wchargin-source: 7a379545e5195047d46d162de36606433a04cf18

wchargin-branch: uploader-graph-filtering wchargin-source: 63adea8ca2f0a85bd9f061aee72435f04d1d56d8 # Conflicts: # tensorboard/uploader/uploader_test.py

wchargin-branch: uploader-graph-filtering wchargin-source: 63adea8ca2f0a85bd9f061aee72435f04d1d56d8

wchargin-branch: efl-compat-transforms wchargin-source: 4fdc21c37611fab6a00384e9e824cd60302f6def

wchargin-branch: efl-compat-transforms wchargin-source: 124d838ae368a9d9c9f061a39a72056554c89790 # Conflicts: # tensorboard/uploader/uploader.py # tensorboard/uploader/uploader_test.py

wchargin-branch: efl-compat-transforms wchargin-source: 124d838ae368a9d9c9f061a39a72056554c89790

wchargin-branch: efl-compat-transforms wchargin-source: 41caa7efc7790458360d47924ce80e0d8f0ef248

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 9bceb864dae1a69432349daf80ebb4db6e19278b

wchargin-branch: audio-no-labels wchargin-source: f0ba8b1511c55b4596450e49d7dbe69d2170fdcc

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: e87f73c1f892413fb38f913c3da0db230f9d8c61 # Conflicts: # tensorboard/backend/event_processing/event_file_loader.py # tensorboard/backend/event_processing/plugin_event_accumulator_test.py # tensorboard/uploader/uploader_test.py

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: e87f73c1f892413fb38f913c3da0db230f9d8c61

wchargin-branch: audio-no-labels wchargin-source: 3ef10438c3ca65d6cff434e277b72807fd27cf36

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: ba0efc9d5159b2f74982e63b6ca884c7f2380495

wchargin-branch: audio-no-labels wchargin-source: dc72cdd9d4dd9383e196b556b6a91917cfd9b7dd # Conflicts: # tensorboard/dataclass_compat.py

wchargin-branch: audio-no-labels wchargin-source: dc72cdd9d4dd9383e196b556b6a91917cfd9b7dd

wchargin-branch: audio-no-labels wchargin-source: adbc9272e97c6ba2a70d84cf055baaab7d166f25 # Conflicts: # tensorboard/dataclass_compat.py

wchargin-branch: audio-no-labels wchargin-source: adbc9272e97c6ba2a70d84cf055baaab7d166f25

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

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

wchargin added plugin:audio core:backend core:summaries labels Apr 9, 2020

wchargin requested a review from nfelt April 9, 2020 18:19

googlebot added the cla: yes label Apr 9, 2020

nfelt approved these changes Apr 9, 2020

View reviewed changes

wchargin added 5 commits April 13, 2020 19:53

[update patch]

61ab333

wchargin-branch: uploader-graph-filtering wchargin-source: 9179acc76158380d9f254df1d0a0aae1ec82df6c

[update diffbase]

45b02e7

wchargin-branch: efl-compat-transforms wchargin-source: 4fa593a34147c7b05c7f4c55f3ce9279242a7414

wchargin changed the base branch from master to wchargin-dataclass-compat-initial-metadata April 14, 2020 03:01

wchargin added 2 commits April 13, 2020 20:01

[update diffbase]

76843be

wchargin-branch: audio-no-labels wchargin-source: 7c7844bff26668d32d692ef7faa0dba3ecbaf138 # Conflicts: # tensorboard/dataclass_compat.py

[update patch]

a12f351

wchargin-branch: audio-no-labels wchargin-source: 7c7844bff26668d32d692ef7faa0dba3ecbaf138

wchargin changed the title ~~audio: remove labels from UI~~ audio: deprecate labels and stop reading them Apr 14, 2020

wchargin commented Apr 14, 2020

View reviewed changes

wchargin added 13 commits April 14, 2020 01:18

[update patch]

8a4247c

wchargin-branch: uploader-graph-filtering wchargin-source: b89f6da9cd740080aefecec49194752b0de85dd4

[bump ci]

aac18b4

wchargin-branch: uploader-graph-filtering wchargin-source: b89f6da9cd740080aefecec49194752b0de85dd4

[update diffbase]

8ef2886

wchargin-branch: efl-compat-transforms wchargin-source: 8abfd3ec5c7394e7b1930b4bf42d2de816e67da4 # Conflicts: # tensorboard/uploader/uploader.py

[update patch]

f159fa5

wchargin-branch: efl-compat-transforms wchargin-source: 8abfd3ec5c7394e7b1930b4bf42d2de816e67da4

[update diffbase]

6d11b7c

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: cfd81441162803b6bab104bc76d93c3b00e8c381

[update diffbase]

6c2d8ae

wchargin-branch: audio-no-labels wchargin-source: eb55e6cf2dcb2dc50a10201296e04c3a030bca3d

[update diffbase]

d17e9f9

wchargin-branch: uploader-graph-filtering wchargin-source: 30f0764698d66cdaf481c797dc56bf6a34ae3ae0

[update diffbase]

84fd685

wchargin-branch: efl-compat-transforms wchargin-source: f18ea5648a96010b48237fe7743e5f5d0d535acb

[update diffbase]

ff15dc7

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 9f516c3b25569b4e007bb929bb456541e5433f52

[update diffbase]

a0edd31

wchargin-branch: audio-no-labels wchargin-source: 7a379545e5195047d46d162de36606433a04cf18

[update diffbase]

94a974c

wchargin-branch: uploader-graph-filtering wchargin-source: 63adea8ca2f0a85bd9f061aee72435f04d1d56d8 # Conflicts: # tensorboard/uploader/uploader_test.py

[update patch]

2ac4935

wchargin-branch: uploader-graph-filtering wchargin-source: 63adea8ca2f0a85bd9f061aee72435f04d1d56d8

wchargin added 13 commits April 14, 2020 12:20

[update diffbase]

7c33f3e

wchargin-branch: efl-compat-transforms wchargin-source: 4fdc21c37611fab6a00384e9e824cd60302f6def

[update diffbase]

a9adf93

wchargin-branch: efl-compat-transforms wchargin-source: 124d838ae368a9d9c9f061a39a72056554c89790 # Conflicts: # tensorboard/uploader/uploader.py # tensorboard/uploader/uploader_test.py

[update patch]

8f469cc

wchargin-branch: efl-compat-transforms wchargin-source: 124d838ae368a9d9c9f061a39a72056554c89790

[update diffbase]

ac58c6d

wchargin-branch: efl-compat-transforms wchargin-source: 41caa7efc7790458360d47924ce80e0d8f0ef248

[update diffbase]

61cc397

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 9bceb864dae1a69432349daf80ebb4db6e19278b

[update patch]

524d0bc

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: 9bceb864dae1a69432349daf80ebb4db6e19278b

[update diffbase]

94e1afe

wchargin-branch: audio-no-labels wchargin-source: f0ba8b1511c55b4596450e49d7dbe69d2170fdcc

[update patch]

d553605

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: e87f73c1f892413fb38f913c3da0db230f9d8c61

[update diffbase]

83b3404

wchargin-branch: audio-no-labels wchargin-source: 3ef10438c3ca65d6cff434e277b72807fd27cf36

[update patch]

3d1d73f

wchargin-branch: dataclass-compat-initial-metadata wchargin-source: ba0efc9d5159b2f74982e63b6ca884c7f2380495

[update diffbase]

0a6d49f

wchargin-branch: audio-no-labels wchargin-source: dc72cdd9d4dd9383e196b556b6a91917cfd9b7dd # Conflicts: # tensorboard/dataclass_compat.py

[update patch]

c1da800

wchargin-branch: audio-no-labels wchargin-source: dc72cdd9d4dd9383e196b556b6a91917cfd9b7dd

wchargin changed the base branch from wchargin-dataclass-compat-initial-metadata to master April 17, 2020 00:57

wchargin added 2 commits April 16, 2020 17:59

[update diffbase]

2b095ce

wchargin-branch: audio-no-labels wchargin-source: adbc9272e97c6ba2a70d84cf055baaab7d166f25 # Conflicts: # tensorboard/dataclass_compat.py

[update patch]

fd201f6

wchargin-branch: audio-no-labels wchargin-source: adbc9272e97c6ba2a70d84cf055baaab7d166f25

wchargin merged commit 0fae2f3 into master Apr 17, 2020

wchargin deleted the wchargin-audio-no-labels branch April 17, 2020 01:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

audio: deprecate labels and stop reading them #3500

audio: deprecate labels and stop reading them #3500

Uh oh!

wchargin commented Apr 9, 2020 •

edited

Loading

Uh oh!

nfelt left a comment

Uh oh!

nfelt Apr 9, 2020

Uh oh!

wchargin Apr 14, 2020

Uh oh!

wchargin left a comment

Uh oh!

wchargin Apr 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants



		_LABELS_WARNING = (
		"Labels on audio summaries are deprecated and will be removed."

audio: deprecate labels and stop reading them #3500

audio: deprecate labels and stop reading them #3500

Uh oh!

Conversation

wchargin commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nfelt left a comment

Choose a reason for hiding this comment

Uh oh!

nfelt Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

wchargin Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

wchargin left a comment

Choose a reason for hiding this comment

Uh oh!

wchargin Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wchargin commented Apr 9, 2020 •

edited

Loading