projector: fix RunPaths result aliasing
#4508
Merged
Conversation
This file contains 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: This rolls forward #4479 (see that commit for context) with changes due to Google-internal, non-TensorBoard code that monkey patches some of the projector internals: - The `_run_paths` field is now updated only during `_update_configs`, to ensure that each change to `_run_paths` gives `_configs` the opportunity to update. - Configs/run paths are no longer updated in `__init__`, since that may be expensive and involve file access (even with a multiplexer). - The `_run_paths` field is now always a `dict` rather than `None`, fixing an existing bug wherein accessing `configs` on a plugin with no multiplexer would raise an `AttributeError`. This reverts commit 4c9a699, and therefore reinstates commit 638014e. Test Plan: Test plan from #4479 still passes, and a test sync now passes as well. Also tested pointing the projector at a logdir with no runs but with checkpoint data at root, as fixed in #3694; this worked in the original version of this change (#4479), but it works now, too. wchargin-branch: projector-fix-aliasing wchargin-source: 0cb7e9c3917d5dc9f00616e35aa5bf756a51dea0
psybuzz
approved these changes
Jan 5, 2021
|
Thanks! No submitted CL (since it was broken). Sent some internal |
wchargin-branch: projector-fix-aliasing wchargin-source: 5fbe2227f0cb997ab814ba974ae6e418ef7423de
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:
This rolls forward #4479 (see that commit for context) with changes due
to Google-internal, non-TensorBoard code that monkey patches some of the
projector internals:
_run_pathsfield is now updated only during_update_configs,to ensure that each change to
_run_pathsgives_configstheopportunity to update.
__init__, since thatmay be expensive and involve file access (even with a multiplexer).
_run_pathsfield is now always adictrather thanNone,fixing an existing bug wherein accessing
configson a plugin withno multiplexer would raise an
AttributeError.This reverts commit 4c9a699, and
therefore reinstates commit 638014e.
Test Plan:
Test plan from #4479 still passes, and a test sync now passes as well.
Also tested pointing the projector at a logdir with no runs but with
checkpoint data at root, as fixed in #3694; this worked in the original
version of this change (#4479), but it works now, too.
Test sync: http://cl/350223680.
wchargin-branch: projector-fix-aliasing