-
Notifications
You must be signed in to change notification settings - Fork 1.7k
backend: move compat transforms to event file loading #3511
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
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: 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
nfelt
reviewed
Apr 14, 2020
There was a problem hiding this 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: 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-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
|
(did you mean to approve?) |
nfelt
approved these changes
Apr 16, 2020
There was a problem hiding this 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.
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
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
data_compatanddataclass_compattransforms are now effected bythe
EventFileLoaderclass. The event accumulator and uploadertherefore 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,graphsstill worksas intended.
wchargin-branch: efl-compat-transforms