Hparams: Change some handling/generation of hparams with discrete domains. (#6489)

This addresses several issues and cleanup related to how we generate and
handle hparams with discrete domains:

1. From the backend, always return the list of discrete domain values
for a string hparam. Previously we were returning no discrete domain
values when the number of values exceeded 10 but this was breaking
assumptions in the Angular hparam logic.
2. In the polymer UI, generate a "regexp" filter for discrete string
hparams with greater than 10 values.
3. Fix regexp filter display logic in the polymer UI. We were never
successfully showing the regexp filter. The check
`[[hparam.filter.regexp]]` was incorrect since it would evaluate to
`false` when `regexp` is the empty string. Instead we must check that
`hparam.filter.regex !== undefined`.
4. Simplify the logic for generating discrete domain filters in the
polymer UI - we just need to do it in a single place now that we can
assume that the backend will always return the list of discrete domain
values (thanks to item (1) and to previous work done in go/tbpr/6393)

Author: bmd3k

tensorboard/plugins/hparams/api.proto

@@ -88,8 +88,7 @@ message HParamInfo {
   // every instance of this hyperparameter will hold a value from this set. It
   // is used by the UI to allow filtering so that only session groups (see
   // below) whose associated hyperparameter value "passes" the filter are
-  // displayed. If this is not populated, the domain is assumed to be the
-  // entire domain of the type of the hyperparameter.
+  // displayed. If this is not populated, the domain is assumed to be empty.
   oneof domain {
     // A discrete set of the values this hyperparameter can hold.
     google.protobuf.ListValue domain_discrete = 5;

tensorboard/plugins/hparams/backend_context.py

@@ -44,18 +44,13 @@ class Context:
     no better place. See http://wiki.c2.com/?MagicContainer
     """
 
-    def __init__(self, tb_context, max_domain_discrete_len=10):
+    def __init__(self, tb_context):
         """Instantiates a context.
 
         Args:
           tb_context: base_plugin.TBContext. The "base" context we extend.
-          max_domain_discrete_len: int. Only used when computing the experiment
-            from the session runs. The maximum number of disticnt values a string
-            hyperparameter can have for us to populate its 'domain_discrete' field.
-            Typically, only tests should specify a value for this parameter.
         """
         self._tb_context = tb_context
-        self._max_domain_discrete_len = max_domain_discrete_len
 
     def experiment_from_metadata(
         self,
@@ -245,8 +240,7 @@ def _compute_hparam_infos(self, hparams_run_to_tag_to_content):
         Finds all the SessionStartInfo messages and collects the hparams values
         appearing in each one. For each hparam attempts to deduce a type that fits
         all its values. Finally, sets the 'domain' of the resulting HParamInfo
-        to be discrete if the type is string and the number of distinct values is
-        small enough.
+        to be discrete if the type is string or boolean.
 
         Returns:
           A list of api_pb2.HParamInfo messages.
@@ -309,12 +303,7 @@ def _compute_hparam_info_from_values(self, name, values):
         if result.type == api_pb2.DATA_TYPE_UNSET:
             return None
 
-        # If the result is a string, set the domain to be the distinct values if
-        # there aren't too many of them.
-        if (
-            result.type == api_pb2.DATA_TYPE_STRING
-            and len(distinct_values) <= self._max_domain_discrete_len
-        ):
+        if result.type == api_pb2.DATA_TYPE_STRING:
             result.domain_discrete.extend(distinct_values)
 
         if result.type == api_pb2.DATA_TYPE_BOOL:

tensorboard/plugins/hparams/backend_context_test.py

@@ -152,11 +152,9 @@ def _mock_list_hyperparameters(
     ):
         return self._hyperparameters
 
-    def _experiment_from_metadata(self, max_domain_discrete_len=10):
+    def _experiment_from_metadata(self):
         """Calls the expected operations for generating an Experiment proto."""
-        ctxt = backend_context.Context(
-            self._mock_tb_context, max_domain_discrete_len
-        )
+        ctxt = backend_context.Context(self._mock_tb_context)
         request_ctx = context.RequestContext()
         return ctxt.experiment_from_metadata(
             request_ctx,
@@ -306,58 +304,6 @@ def test_experiment_without_experiment_tag_different_hparam_types(self):
         _canonicalize_experiment(actual_exp)
         self.assertProtoEquals(expected_exp, actual_exp)
 
-    def test_experiment_without_experiment_tag_many_distinct_values(self):
-        self.session_1_start_info_ = """
-            hparams:[
-              {key: 'batch_size' value: {number_value: 100}},
-              {key: 'lr' value: {string_value: '0.01'}}
-            ]
-        """
-        self.session_2_start_info_ = """
-            hparams:[
-              {key: 'lr' value: {number_value: 0.02}},
-              {key: 'model_type' value: {string_value: 'CNN'}}
-            ]
-        """
-        self.session_3_start_info_ = """
-            hparams:[
-              {key: 'batch_size' value: {bool_value: true}},
-              {key: 'model_type' value: {string_value: 'CNN'}}
-            ]
-        """
-        expected_exp = """
-            hparam_infos: {
-              name: 'batch_size'
-              type: DATA_TYPE_STRING
-            }
-            hparam_infos: {
-              name: 'lr'
-              type: DATA_TYPE_STRING
-            }
-            hparam_infos: {
-              name: 'model_type'
-              type: DATA_TYPE_STRING
-              domain_discrete: {
-                values: [{string_value: 'CNN'}]
-              }
-            }
-            metric_infos: {
-              name: {group: '', tag: 'accuracy'}
-            }
-            metric_infos: {
-              name: {group: '', tag: 'loss'}
-            }
-            metric_infos: {
-              name: {group: 'eval', tag: 'loss'}
-            }
-            metric_infos: {
-              name: {group: 'train', tag: 'loss'}
-            }
-        """
-        actual_exp = self._experiment_from_metadata(max_domain_discrete_len=1)
-        _canonicalize_experiment(actual_exp)
-        self.assertProtoEquals(expected_exp, actual_exp)
-
     def test_experiment_with_bool_types(self):
         self.session_1_start_info_ = """
             hparams:[

tensorboard/plugins/hparams/tf_hparams_query_pane/tf-hparams-query-pane.ts

@@ -41,6 +41,8 @@ interface ColumnMetric {
   order?: string;
 }
 
+const MAX_DOMAIN_DISCRETE_LIST_LEN = 10;
+
 /**
  * The tf-hparams-query-pane element implements controls for querying the
  * server for a list of session groups. It provides filtering, and
@@ -102,7 +104,7 @@ class TfHparamsQueryPane extends LegacyElementMixin(PolymerElement) {
               </paper-input>
             </template>
             <!-- 3. A regexp -->
-            <template is="dom-if" if="[[hparam.filter.regexp]]">
+            <template is="dom-if" if="[[_hasRegexpFilter(hparam)]]">
               <paper-input
                 label="Regular expression"
                 value="{{hparam.filter.regexp}}"
@@ -581,25 +583,22 @@ class TfHparamsQueryPane extends LegacyElementMixin(PolymerElement) {
         filter: {} as any,
       };
       if (hparam.info.hasOwnProperty('domainDiscrete')) {
-        hparam.filter.domainDiscrete = [];
-        hparam.info.domainDiscrete.forEach((val) => {
-          hparam.filter.domainDiscrete.push({
-            value: val,
-            checked: true,
+        // Handle a discrete domain. Could be of any data type.
+        if (hparam.info.domainDiscrete.length < MAX_DOMAIN_DISCRETE_LIST_LEN) {
+          hparam.filter.domainDiscrete = [];
+          hparam.info.domainDiscrete.forEach((val: any) => {
+            hparam.filter.domainDiscrete.push({
+              value: val,
+              checked: true,
+            });
           });
-        });
-      } else if (hparam.info.type === 'DATA_TYPE_BOOL') {
-        hparam.filter.domainDiscrete = [
-          {
-            value: false,
-            checked: true,
-          },
-          {
-            value: true,
-            checked: true,
-          },
-        ];
+        } else {
+          // Don't show long lists of values. If the list surpasses a certain
+          // threshold then the user instead specifies regex filters.
+          hparam.filter.regexp = '';
+        }
       } else if (hparam.info.type === 'DATA_TYPE_FLOAT64') {
+        // Handle a float interval domain.
         hparam.filter.interval = {
           min: {
             value: '',
@@ -610,10 +609,11 @@ class TfHparamsQueryPane extends LegacyElementMixin(PolymerElement) {
             invalid: false,
           },
         };
-      } else if (hparam.info.type === 'DATA_TYPE_STRING') {
-        hparam.filter.regexp = '';
       } else {
-        console.warn('unknown hparam.info.type: %s', hparam.info.type);
+        console.warn(
+          'cannot process domain type %s without discrete domain values',
+          hparam.info.type
+        );
       }
       result.push(hparam);
     });
@@ -690,6 +690,10 @@ class TfHparamsQueryPane extends LegacyElementMixin(PolymerElement) {
       metricInfos: newMetricInfos,
     };
   }
+  // Determines if a regex filter should be rendered.
+  _hasRegexpFilter(hparam) {
+    return hparam.filter.regexp !== undefined;
+  }
   // Sends a query to the server for the list of session groups.
   // Asynchronously updates the sessionGroups property with the response.
   _queryServer() {