From 52e93033d48e8d35119a8da73bc338764d96c95f Mon Sep 17 00:00:00 2001 From: Yating Jing Date: Thu, 4 Apr 2024 20:17:01 +0000 Subject: [PATCH 1/2] support loading metrics from runs whoses names are `.` --- tensorboard/plugins/hparams/backend_context.py | 17 +++++++---------- .../plugins/hparams/backend_context_test.py | 9 +++++++++ tensorboard/plugins/hparams/metrics.py | 4 +++- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/tensorboard/plugins/hparams/backend_context.py b/tensorboard/plugins/hparams/backend_context.py index 5a403062f9..2a70279774 100644 --- a/tensorboard/plugins/hparams/backend_context.py +++ b/tensorboard/plugins/hparams/backend_context.py @@ -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.""" @@ -494,9 +487,13 @@ def _compute_metric_names(self, ctx, experiment_id, session_runs): if session is None: continue group = os.path.relpath(run, session) + # Do not normalize the path since the run name could be ".", note that + # `os.path.relpath` would remove the the trailing "/.". + if run.endswith("/."): + group += "/." # relpath() returns "." for the 'session' directory, we use an empty - # string. - if group == ".": + # string, unless the run name 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) diff --git a/tensorboard/plugins/hparams/backend_context_test.py b/tensorboard/plugins/hparams/backend_context_test.py index 6d8eff0cd0..da89ab76be 100644 --- a/tensorboard/plugins/hparams/backend_context_test.py +++ b/tensorboard/plugins/hparams/backend_context_test.py @@ -129,6 +129,9 @@ def _mock_list_scalars( "exp/session_3xyz/": { "loss2": b"", }, + "exp/session_4/.": { + "entropy": b"", + }, } result = {} for run, tag_to_content in scalars_content.items(): @@ -900,6 +903,12 @@ def test_experiment_from_data_provider_session_group_without_session_names( tag: "accuracy" } } + metric_infos { + name { + group: "exp/session_4/." + tag: "entropy" + } + } metric_infos { name { group: "exp/session_1" diff --git a/tensorboard/plugins/hparams/metrics.py b/tensorboard/plugins/hparams/metrics.py index 4155c6cd85..02ade47eef 100644 --- a/tensorboard/plugins/hparams/metrics.py +++ b/tensorboard/plugins/hparams/metrics.py @@ -32,10 +32,12 @@ def run_tag_from_session_and_metric(session_name, metric_name): """ assert isinstance(session_name, str) assert isinstance(metric_name, api_pb2.MetricName) + run = os.path.join(session_name, metric_name.group) # 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)) + if run.endswith("/"): + run = run[:-1] tag = metric_name.tag return run, tag From d98e7db2c759b784ae0f4338988a23f90080586a Mon Sep 17 00:00:00 2001 From: Yating Jing Date: Thu, 4 Apr 2024 20:55:56 +0000 Subject: [PATCH 2/2] format metric name on the FE --- tensorboard/plugins/hparams/backend_context.py | 6 +----- tensorboard/plugins/hparams/backend_context_test.py | 4 ++-- tensorboard/plugins/hparams/metrics.py | 5 ++--- .../plugins/hparams/tf_hparams_utils/tf-hparams-utils.ts | 2 +- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tensorboard/plugins/hparams/backend_context.py b/tensorboard/plugins/hparams/backend_context.py index 2a70279774..4535d44951 100644 --- a/tensorboard/plugins/hparams/backend_context.py +++ b/tensorboard/plugins/hparams/backend_context.py @@ -487,12 +487,8 @@ def _compute_metric_names(self, ctx, experiment_id, session_runs): if session is None: continue group = os.path.relpath(run, session) - # Do not normalize the path since the run name could be ".", note that - # `os.path.relpath` would remove the the trailing "/.". - if run.endswith("/."): - group += "/." # relpath() returns "." for the 'session' directory, we use an empty - # string, unless the run name ends with ".". + # 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) diff --git a/tensorboard/plugins/hparams/backend_context_test.py b/tensorboard/plugins/hparams/backend_context_test.py index da89ab76be..ba530c1c9c 100644 --- a/tensorboard/plugins/hparams/backend_context_test.py +++ b/tensorboard/plugins/hparams/backend_context_test.py @@ -129,7 +129,7 @@ def _mock_list_scalars( "exp/session_3xyz/": { "loss2": b"", }, - "exp/session_4/.": { + ".": { "entropy": b"", }, } @@ -905,7 +905,7 @@ def test_experiment_from_data_provider_session_group_without_session_names( } metric_infos { name { - group: "exp/session_4/." + group: "." tag: "entropy" } } diff --git a/tensorboard/plugins/hparams/metrics.py b/tensorboard/plugins/hparams/metrics.py index 02ade47eef..435c40321e 100644 --- a/tensorboard/plugins/hparams/metrics.py +++ b/tensorboard/plugins/hparams/metrics.py @@ -32,11 +32,10 @@ def run_tag_from_session_and_metric(session_name, metric_name): """ assert isinstance(session_name, str) assert isinstance(metric_name, api_pb2.MetricName) - run = os.path.join(session_name, metric_name.group) # 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. + # 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 diff --git a/tensorboard/plugins/hparams/tf_hparams_utils/tf-hparams-utils.ts b/tensorboard/plugins/hparams/tf_hparams_utils/tf-hparams-utils.ts index f51728cf56..39e288f5c1 100644 --- a/tensorboard/plugins/hparams/tf_hparams_utils/tf-hparams-utils.ts +++ b/tensorboard/plugins/hparams/tf_hparams_utils/tf-hparams-utils.ts @@ -75,7 +75,7 @@ export function metricName(metricInfo) { if (tag === undefined) { tag = ''; } - if (group === '') { + if (group === '' || group === '.') { return tag; } return group + '.' + tag;