Skip to content

Commit

Permalink
Hparams: fix metrics loading when group name is "." (tensorflow#6822)
Browse files Browse the repository at this point in the history
In the Hparams plugin, when fetching metrics (scalar data) for multiple experiments in the comparison view, we can't replace group name `.` with an empty string.

Googlers, see b/332308243 for more details.

Tested internally: cl/582334596

#hparams #oncall
  • Loading branch information
yatbear authored and AnuarTB committed May 3, 2024
1 parent a8eb83b commit 9da01a4
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 14 deletions.
13 changes: 3 additions & 10 deletions tensorboard/plugins/hparams/backend_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,12 @@ def read_last_scalars(self, ctx, experiment_id, run_tag_filter):
value, with keys only for runs and tags that actually had
data, which may be a subset of what was requested.
"""
last_scalars = self._tb_context.data_provider.read_last_scalars(
return self._tb_context.data_provider.read_last_scalars(
ctx,
experiment_id=experiment_id,
plugin_name=scalar_metadata.PLUGIN_NAME,
run_tag_filter=run_tag_filter,
)
# Transform keys from the data provider using some of the same os-level
# filesystem operations used to generate metric names elsewhere.
# This will, for example, translate runs with name 'SOMETHING/.' to
# 'SOMETHING'.
return {
os.path.normpath(key): value for key, value in last_scalars.items()
}

def hparams_from_data_provider(self, ctx, experiment_id, limit):
"""Calls DataProvider.list_hyperparameters() and returns the result."""
Expand Down Expand Up @@ -495,8 +488,8 @@ def _compute_metric_names(self, ctx, experiment_id, session_runs):
continue
group = os.path.relpath(run, session)
# relpath() returns "." for the 'session' directory, we use an empty
# string.
if group == ".":
# string, unless the run name actually ends with ".".
if group == "." and not run.endswith("."):
group = ""
metric_names_set.update((tag, group) for tag in tags)
metric_names_list = list(metric_names_set)
Expand Down
9 changes: 9 additions & 0 deletions tensorboard/plugins/hparams/backend_context_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ def _mock_list_scalars(
"exp/session_3xyz/": {
"loss2": b"",
},
".": {
"entropy": b"",
},
}
result = {}
for run, tag_to_content in scalars_content.items():
Expand Down Expand Up @@ -900,6 +903,12 @@ def test_experiment_from_data_provider_session_group_without_session_names(
tag: "accuracy"
}
}
metric_infos {
name {
group: "."
tag: "entropy"
}
}
metric_infos {
name {
group: "exp/session_1"
Expand Down
7 changes: 4 additions & 3 deletions tensorboard/plugins/hparams/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ def run_tag_from_session_and_metric(session_name, metric_name):
assert isinstance(metric_name, api_pb2.MetricName)
# os.path.join() will append a final slash if the group is empty; it seems
# like multiplexer.Tensors won't recognize paths that end with a '/' so
# we normalize the result of os.path.join() to remove the final '/' in that
# case.
run = os.path.normpath(os.path.join(session_name, metric_name.group))
# we remove the final '/' in that case.
run = os.path.join(session_name, metric_name.group)
if run.endswith("/"):
run = run[:-1]
tag = metric_name.tag
return run, tag
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function metricName(metricInfo) {
if (tag === undefined) {
tag = '';
}
if (group === '') {
if (group === '' || group === '.') {
return tag;
}
return group + '.' + tag;
Expand Down

0 comments on commit 9da01a4

Please sign in to comment.