Skip to content

Commit

Permalink
projector: fix RunPaths result aliasing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wchargin committed Dec 23, 2020
1 parent 4c9a699 commit b52d274
Showing 1 changed file with 33 additions and 38 deletions.
71 changes: 33 additions & 38 deletions tensorboard/plugins/projector/projector_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,8 @@ def __init__(self, context):
self.multiplexer = context.multiplexer
self.logdir = context.logdir
self.readers = {}
self.run_paths = None
self._run_paths = None
self._configs = {}
self.old_num_run_paths = None
self.config_fpaths = None
self.tensor_cache = LRUCache(_TENSOR_CACHE_CAPACITY)

Expand All @@ -257,9 +256,6 @@ def __init__(self, context):
# active. If such a thread exists, do not start a duplicate thread.
self._thread_for_determining_is_active = None

if self.multiplexer:
self.run_paths = self.multiplexer.RunPaths()

def get_plugin_apps(self):
asset_prefix = "tf_projector_plugin"
return {
Expand Down Expand Up @@ -329,37 +325,36 @@ def _determine_is_active(self):
offer an immediate response to whether it is active and
determine whether it should be active in a separate thread.
"""
if self.configs:
self._update_configs()
if self._configs:
self._is_active = True
self._thread_for_determining_is_active = None

@property
def configs(self):
"""Returns a map of run paths to `ProjectorConfig` protos."""
run_path_pairs = list(self.run_paths.items())
def _update_configs(self):
"""Updates `self._configs` and `self._run_paths`."""
if self.multiplexer:
run_paths = dict(self.multiplexer.RunPaths())
else:
run_paths = {}
run_paths_changed = run_paths != self._run_paths
self._run_paths = run_paths

run_path_pairs = list(self._run_paths.items())
self._append_plugin_asset_directories(run_path_pairs)
# Also accept the root logdir as a model checkpoint directory,
# so that the projector still works when there are no runs.
# (Case on `run` rather than `path` to avoid issues with
# absolute/relative paths on any filesystems.)
if not any(run == "." for (run, path) in run_path_pairs):
if "." not in self._run_paths:
run_path_pairs.append((".", self.logdir))
if self._run_paths_changed() or _latest_checkpoints_changed(
if run_paths_changed or _latest_checkpoints_changed(
self._configs, run_path_pairs
):
self.readers = {}
self._configs, self.config_fpaths = self._read_latest_config_files(
run_path_pairs
)
self._augment_configs_with_checkpoint_info()
return self._configs

def _run_paths_changed(self):
num_run_paths = len(list(self.run_paths.keys()))
if num_run_paths != self.old_num_run_paths:
self.old_num_run_paths = num_run_paths
return True
return False

def _augment_configs_with_checkpoint_info(self):
for run, config in self._configs.items():
Expand Down Expand Up @@ -524,7 +519,7 @@ def _append_plugin_asset_directories(self, run_path_pairs):
if metadata.PROJECTOR_FILENAME not in assets:
continue
assets_dir = os.path.join(
self.run_paths[run],
self._run_paths[run],
metadata.PLUGINS_DIR,
metadata.PLUGIN_ASSETS_NAME,
)
Expand All @@ -542,7 +537,8 @@ def _serve_file(self, file_path, request):
@wrappers.Request.application
def _serve_runs(self, request):
"""Returns a list of runs that have embeddings."""
return Respond(request, list(self.configs.keys()), "application/json")
self._update_configs()
return Respond(request, list(self._configs.keys()), "application/json")

@wrappers.Request.application
def _serve_config(self, request):
Expand All @@ -551,12 +547,12 @@ def _serve_config(self, request):
return Respond(
request, 'query parameter "run" is required', "text/plain", 400
)
if run not in self.configs:
self._update_configs()
config = self._configs.get(run)
if config is None:
return Respond(
request, 'Unknown run: "%s"' % run, "text/plain", 400
)

config = self.configs[run]
return Respond(
request, json_format.MessageToJson(config), "application/json"
)
Expand Down Expand Up @@ -584,12 +580,12 @@ def _serve_metadata(self, request):
400,
)

if run not in self.configs:
self._update_configs()
config = self._configs.get(run)
if config is None:
return Respond(
request, 'Unknown run: "%s"' % run, "text/plain", 400
)

config = self.configs[run]
fpath = self._get_metadata_file_for_tensor(name, config)
if not fpath:
return Respond(
Expand Down Expand Up @@ -644,13 +640,12 @@ def _serve_tensor(self, request):
400,
)

if run not in self.configs:
self._update_configs()
config = self._configs.get(run)
if config is None:
return Respond(
request, 'Unknown run: "%s"' % run, "text/plain", 400
)

config = self.configs[run]

tensor = self.tensor_cache.get((run, name))
if tensor is None:
# See if there is a tensor file in the config.
Expand Down Expand Up @@ -711,12 +706,12 @@ def _serve_bookmarks(self, request):
request, 'query parameter "name" is required', "text/plain", 400
)

if run not in self.configs:
self._update_configs()
config = self._configs.get(run)
if config is None:
return Respond(
request, 'Unknown run: "%s"' % run, "text/plain", 400
)

config = self.configs[run]
fpath = self._get_bookmarks_file_for_tensor(name, config)
if not fpath:
return Respond(
Expand Down Expand Up @@ -754,14 +749,14 @@ def _serve_sprite_image(self, request):
request, 'query parameter "name" is required', "text/plain", 400
)

if run not in self.configs:
self._update_configs()
config = self._configs.get(run)
if config is None:
return Respond(
request, 'Unknown run: "%s"' % run, "text/plain", 400
)

config = self.configs[run]
embedding_info = self._get_embedding(name, config)

if not embedding_info or not embedding_info.sprite.image_path:
return Respond(
request,
Expand Down

0 comments on commit b52d274

Please sign in to comment.