KEMBAR78
backend: move compat transforms to event file loading by wchargin · Pull Request #3511 · tensorflow/tensorboard · GitHub
Skip to content

Conversation

@wchargin
Copy link
Contributor

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

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
wchargin-branch: uploader-graph-filtering
wchargin-source: 9179acc76158380d9f254df1d0a0aae1ec82df6c
wchargin-branch: efl-compat-transforms
wchargin-source: 4fa593a34147c7b05c7f4c55f3ce9279242a7414
Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, looks like this ended up working out nicely :)

wchargin-branch: uploader-graph-filtering
wchargin-source: b89f6da9cd740080aefecec49194752b0de85dd4
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
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: 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 wchargin changed the base branch from wchargin-uploader-graph-filtering to master April 14, 2020 19:39
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 wchargin requested a review from nfelt April 16, 2020 17:49
@wchargin
Copy link
Contributor Author

(did you mean to approve?)

Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry! Yes I did.

@wchargin wchargin merged commit 68beffa into master Apr 16, 2020
@wchargin wchargin deleted the wchargin-efl-compat-transforms branch April 16, 2020 22:37
caisq pushed a commit to caisq/tensorboard that referenced this pull request May 19, 2020
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
caisq pushed a commit that referenced this pull request May 27, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants