Hparams: Sort by differs then by name. (#6601)

After sorting with the explicitly specified sorting key (`differs`), `sorted()` returns the order within each `differs` group differently when running externally v.s. internally, which broke internal tests. Changing to sort by name within each `differs` group as a workaround. Googlers, see the test failure details in cl/568227790. #hparams
tensorflow · Sep 25, 2023 · 9394c5e · 9394c5e
1 parent 3584a84
commit 9394c5e
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 21 deletions.
diff --git a/tensorboard/plugins/hparams/backend_context.py b/tensorboard/plugins/hparams/backend_context.py
@@ -605,10 +605,10 @@ def _sort_and_reduce_to_hparams_limit(experiment, hparams_limit=None):
         hparams_limit = len(experiment.hparam_infos)
 
     # Prioritizes returning HParamInfo protos with `differed` values.
+    # Sorts by `differs` (True first), then by name.
     limited_hparam_infos = sorted(
         experiment.hparam_infos,
-        key=lambda hparam_info: hparam_info.differs,
-        reverse=True,
+        key=lambda hparam_info: (not hparam_info.differs, hparam_info.name),
     )[:hparams_limit]
 
     experiment.ClearField("hparam_infos")

diff --git a/tensorboard/plugins/hparams/backend_context_test.py b/tensorboard/plugins/hparams/backend_context_test.py
@@ -1188,31 +1188,32 @@ def test_experiment_from_tags_sorts_differed_hparams_first(self):
     def test_experiment_from_runs_with_hparams_limit_no_differed_hparams(self):
         self.session_1_start_info_ = """
             hparams: [
-              {key: 'lr' value: {number_value: 100}},
+              {key: 'lr' value: {number_value: 0.001}},
               {key: 'model_type' value: {string_value: 'LATTICE'}},
               {key: 'use_batch_norm' value: {bool_value: true}}
             ]
         """
         self.session_2_start_info_ = """
             hparams: [
-              {key: 'lr' value: {number_value: 100}},
+              {key: 'lr' value: {number_value: 0.001}},
               {key: 'model_type' value: {string_value: 'LATTICE'}},
               {key: 'use_batch_norm' value: {bool_value: true}}
             ]
         """
         self.session_3_start_info_ = """
             hparams: [
-              {key: 'lr' value: {number_value: 100}},
+              {key: 'lr' value: {number_value: 0.001}},
               {key: 'model_type' value: {string_value: 'LATTICE'}},
               {key: 'use_batch_norm' value: {bool_value: true}}
             ]
         """
         expected_exp = """
             hparam_infos: {
-              name: 'use_batch_norm'
-              type: DATA_TYPE_BOOL
-              domain_discrete: {
-                values: [{bool_value: true}]
+              name: 'lr'
+              type: DATA_TYPE_FLOAT64
+              domain_interval {
+                min_value: 0.001
+                max_value: 0.001
               }
               differs: false
             }
@@ -1306,14 +1307,6 @@ def test_experiment_from_runs_sorts_differed_hparams_first(self):
             ]
         """
         expected_exp = """
-            hparam_infos: {
-              name: 'use_batch_norm'
-              type: DATA_TYPE_BOOL
-              domain_discrete: {
-                values: [{bool_value: false}, {bool_value: true}]
-              }
-              differs: true
-            }
             hparam_infos: {
               name: 'batch_size'
               type: DATA_TYPE_FLOAT64
@@ -1324,12 +1317,12 @@ def test_experiment_from_runs_sorts_differed_hparams_first(self):
               differs: true
             }
             hparam_infos: {
-              name: 'model_type'
-              type: DATA_TYPE_STRING
+              name: 'use_batch_norm'
+              type: DATA_TYPE_BOOL
               domain_discrete: {
-                values: [{string_value: 'CNN'}]
+                values: [{bool_value: false}, {bool_value: true}]
               }
-              differs: false
+              differs: true
             }
             hparam_infos: {
               name: 'lr'
@@ -1340,6 +1333,14 @@ def test_experiment_from_runs_sorts_differed_hparams_first(self):
               }
               differs: false
             }
+            hparam_infos: {
+              name: 'model_type'
+              type: DATA_TYPE_STRING
+              domain_discrete: {
+                values: [{string_value: 'CNN'}]
+              }
+              differs: false
+            }
         """
         actual_exp = self._experiment_from_metadata(
             include_metrics=False, hparams_limit=None