From 6627a7fbd1bffd6d08a0cf26c5960d211e3eea0d Mon Sep 17 00:00:00 2001 From: Brian Dubois Date: Wed, 16 Aug 2023 11:36:16 -0400 Subject: [PATCH] Hparams: Allow sessions without name to pull all run,tag combinations as metrics. (#6546) Motivation: Sometimes a user is viewing a single experiment and the DataProvider determines that experiment contains exactly one session. In this case the DataProvider will return a session with both empty experiment_id and empty run. We want this "empty-name session" to represent all runs in the experiment. But we have a bug where the /experiment and /session_group responses do not contain any metrics for the session. We want to instead designate all (run,tag) combinations as separate metrics to include in the /experiment and /session_groups responses. The first change to get this to work: * When generating session names, a session with experiment_id="" and run="" should generate an empty session_name "" (instead of using the somewhat-meaningless input experiment id as the session_name). The side-effect of the first change is that calls to _find_longest_parent_path() will return "" for all runs passed into it - effectively saying that all runs belong to the "empty-name session". So the second change to get this to work: * When _find_longest_parent_path() returns "" instead of None, treat this as a match with "empty-name session" instead of ignoring it. --- .../plugins/hparams/backend_context.py | 8 +- .../plugins/hparams/backend_context_test.py | 100 ++++++++++++++++++ .../plugins/hparams/list_session_groups.py | 6 +- .../hparams/list_session_groups_test.py | 33 ++++++ 4 files changed, 141 insertions(+), 6 deletions(-) diff --git a/tensorboard/plugins/hparams/backend_context.py b/tensorboard/plugins/hparams/backend_context.py index 5a61939d9f5..6eab89a62e8 100644 --- a/tensorboard/plugins/hparams/backend_context.py +++ b/tensorboard/plugins/hparams/backend_context.py @@ -403,7 +403,7 @@ def compute_metric_infos_from_data_provider_session_groups( self, ctx, experiment_id, session_groups ): session_runs = set( - generate_data_provider_session_name(experiment_id, s) + generate_data_provider_session_name(s) for sg in session_groups for s in sg.sessions ) @@ -446,7 +446,7 @@ def _compute_metric_names(self, ctx, experiment_id, session_runs): ) for run, tags in scalars_run_to_tag_to_content.items(): session = _find_longest_parent_path(session_runs, run) - if not session: + if session is None: continue group = os.path.relpath(run, session) # relpath() returns "." for the 'session' directory, we use an empty @@ -460,14 +460,14 @@ def _compute_metric_names(self, ctx, experiment_id, session_runs): return metric_names_list -def generate_data_provider_session_name(experiment_id, session): +def generate_data_provider_session_name(session): """Generates a name from a HyperparameterSesssionRun. If the HyperparameterSessionRun contains no experiment or run information then the name is set to the original experiment_id. """ if not session.experiment_id and not session.run: - return experiment_id + return "" elif not session.experiment_id: return session.run elif not session.run: diff --git a/tensorboard/plugins/hparams/backend_context_test.py b/tensorboard/plugins/hparams/backend_context_test.py index ef7d6f36a0e..784ce9b1589 100644 --- a/tensorboard/plugins/hparams/backend_context_test.py +++ b/tensorboard/plugins/hparams/backend_context_test.py @@ -689,6 +689,106 @@ def test_experiment_from_data_provider_session_group_without_experiment_name( """ self.assertProtoEquals(expected_exp, actual_exp) + def test_experiment_from_data_provider_session_group_without_session_names( + self, + ): + self._mock_tb_context.data_provider.list_tensors.side_effect = None + self._hyperparameters = provider.ListHyperparametersResult( + hyperparameters=[], + session_groups=[ + provider.HyperparameterSessionGroup( + root=provider.HyperparameterSessionRun( + experiment_id="", run="" + ), + sessions=[ + provider.HyperparameterSessionRun( + experiment_id="", run="" + ), + ], + hyperparameter_values=[], + ), + ], + ) + actual_exp = self._experiment_from_metadata() + # The result specifies a single session without explicit identifier. It + # therefore represents a session that includes all run/tag combinations + # as separate metric values. + expected_exp = """ + metric_infos { + name { + group: "exp/session_1" + tag: "accuracy" + } + } + metric_infos { + name { + group: "exp/session_2" + tag: "accuracy" + } + } + metric_infos { + name { + group: "exp/session_3" + tag: "accuracy" + } + } + metric_infos { + name { + group: "exp/session_1" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_1/eval" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_1/train" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_2" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_2/eval" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_2/train" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_3" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_3/eval" + tag: "loss" + } + } + metric_infos { + name { + group: "exp/session_3xyz" + tag: "loss2" + } + } + """ + self.assertProtoEquals(expected_exp, actual_exp) + def test_experiment_from_data_provider_old_response_type(self): self._hyperparameters = [ provider.Hyperparameter( diff --git a/tensorboard/plugins/hparams/list_session_groups.py b/tensorboard/plugins/hparams/list_session_groups.py index 8252a76258b..31aa71ce5c7 100644 --- a/tensorboard/plugins/hparams/list_session_groups.py +++ b/tensorboard/plugins/hparams/list_session_groups.py @@ -136,7 +136,7 @@ def _session_groups_from_data_provider(self): for session in provider_group.sessions: session_name = ( backend_context_lib.generate_data_provider_session_name( - self._experiment_id, session + session ) ) sessions.append( @@ -150,8 +150,10 @@ def _session_groups_from_data_provider(self): ) name = backend_context_lib.generate_data_provider_session_name( - self._experiment_id, provider_group.root + provider_group.root ) + if not name: + name = self._experiment_id session_group = api_pb2.SessionGroup( name=name, sessions=sessions, diff --git a/tensorboard/plugins/hparams/list_session_groups_test.py b/tensorboard/plugins/hparams/list_session_groups_test.py index 34b486ccedd..3cb150d8328 100644 --- a/tensorboard/plugins/hparams/list_session_groups_test.py +++ b/tensorboard/plugins/hparams/list_session_groups_test.py @@ -2066,6 +2066,39 @@ def test_experiment_from_data_provider_with_metric_values_from_run_name( response.session_groups[0].sessions[0], ) + def test_experiment_from_data_provider_with_metric_values_empty_session_names( + self, + ): + self._mock_tb_context.data_provider.list_tensors.side_effect = None + self._hyperparameters = [ + provider.HyperparameterSessionGroup( + root=provider.HyperparameterSessionRun( + experiment_id="", run="" + ), + sessions=[ + provider.HyperparameterSessionRun(experiment_id="", run="") + ], + hyperparameter_values=[], + ), + ] + request = """ + start_index: 0 + slice_size: 10 + """ + response = self._run_handler(request) + self.assertLen(response.session_groups, 1) + # The name comes from the experiment id. + self.assertEquals(response.session_groups[0].name, "123") + self.assertLen(response.session_groups[0].sessions, 1) + self.assertEquals(response.session_groups[0].sessions[0].name, "") + # The result specifies a single session without explicit identifier. It + # therefore represents a session that includes all run/tag combinations + # as separate metric values. + # There are 11 total run/tag combinations across session_1, _2, _3, _4, + # and _5. + self.assertLen(response.session_groups[0].metric_values, 11) + self.assertLen(response.session_groups[0].sessions[0].metric_values, 11) + def test_experiment_from_data_provider_with_metric_values_aggregates( self, ):