HParam: Stop including default filters by default (#6525)

## Motivation for features / changes
Default hparam and metric filters has lead to a lot of subtle bugs when
values are not as expected

Examples of such errors:
NaN values lead to all runs being filtered out.
Booleans have been treated as strings without domains and thus are
filtered out

Because the existing runs table uses these default filters I am opting
not to outright remove them but rather to only read them if an
additional parameter is provided to the new selector.

Author: rileyajones

tensorboard/webapp/hparams/_redux/hparams_selectors.ts

@@ -76,14 +76,17 @@ export const getHparamFilterMap = createSelector(
   getHparamFilterMapResultFunction
 );
 
-export function getHparamFilterMapFromExperimentIds(experimentIds: string[]) {
+export function getHparamFilterMapFromExperimentIds(
+  experimentIds: string[],
+  includeDefaults?: boolean
+) {
   return createSelector(
     getHparamsState,
     getHparamsDefaultFiltersForExperimentsFromExperimentIds(experimentIds),
-    (hparamState, combinedDefaultFilterMap) =>
+    (hparamState, defaultFilterMap) =>
       getHparamFilterMapResultFunction(
         hparamState,
-        combinedDefaultFilterMap,
+        includeDefaults ? defaultFilterMap : new Map(),
         experimentIds
       )
   );
@@ -136,12 +139,19 @@ export const getMetricFilterMap = createSelector(
   getMetricFilterMapResultFunction
 );
 
-export function getMetricFilterMapFromExperimentIds(experimentIds: string[]) {
+export function getMetricFilterMapFromExperimentIds(
+  experimentIds: string[],
+  includeDefaults?: boolean
+) {
   return createSelector(
     getHparamsState,
     getMetricsDefaultFiltersForExperimentsFromExperimentIds(experimentIds),
     (state, defaultFilterMap) =>
-      getMetricFilterMapResultFunction(state, defaultFilterMap, experimentIds)
+      getMetricFilterMapResultFunction(
+        state,
+        includeDefaults ? defaultFilterMap : new Map(),
+        experimentIds
+      )
   );
 }
 

tensorboard/webapp/hparams/_redux/hparams_selectors_test.ts

@@ -118,7 +118,7 @@ describe('hparams/_redux/hparams_selectors_test', () => {
   });
 
   describe('#getHparamFilterMapFromExperimentIds()', () => {
-    it('returns default hparam filter map', () => {
+    it('returns default hparam filter map when includeDefaults is true', () => {
       const state = buildStateFromHparamsState(
         buildHparamsState(
           buildSpecs('foo', {
@@ -138,7 +138,7 @@ describe('hparams/_redux/hparams_selectors_test', () => {
       );
 
       expect(
-        selectors.getHparamFilterMapFromExperimentIds(['foo'])(state)
+        selectors.getHparamFilterMapFromExperimentIds(['foo'], true)(state)
       ).toEqual(
         new Map([
           ['optimizer', buildDiscreteFilter({filterValues: ['a', 'b', 'c']})],
@@ -207,6 +207,30 @@ describe('hparams/_redux/hparams_selectors_test', () => {
         selectors.getHparamFilterMapFromExperimentIds(['bar'])(state)
       ).toEqual(new Map());
     });
+
+    it('does not use default filters when includeDefaults is false', () => {
+      const state = buildStateFromHparamsState(
+        buildHparamsState(
+          buildSpecs('foo', {
+            hparam: {
+              specs: [buildHparamSpec({name: 'optimizer'})],
+              defaultFilters: new Map([
+                [
+                  'optimizer',
+                  buildDiscreteFilter({
+                    filterValues: ['a', 'b', 'c'],
+                  }),
+                ],
+              ]),
+            },
+          })
+        )
+      );
+
+      expect(
+        selectors.getHparamFilterMapFromExperimentIds(['foo'], false)(state)
+      ).toEqual(new Map([]));
+    });
   });
 
   describe('#getMetricFilterMap', () => {
@@ -326,7 +350,7 @@ describe('hparams/_redux/hparams_selectors_test', () => {
   });
 
   describe('#getMetricFilterMapFromExperimentIds', () => {
-    it('returns default metric filter map', () => {
+    it('returns default metric filter map  when includeDefaults is true', () => {
       const state = buildStateFromHparamsState(
         buildHparamsState(
           buildSpecs('foo', {
@@ -347,7 +371,7 @@ describe('hparams/_redux/hparams_selectors_test', () => {
       );
 
       expect(
-        selectors.getMetricFilterMapFromExperimentIds(['foo'])(state)
+        selectors.getMetricFilterMapFromExperimentIds(['foo'], true)(state)
       ).toEqual(
         new Map([
           [
@@ -441,4 +465,30 @@ describe('hparams/_redux/hparams_selectors_test', () => {
       ).toEqual(new Map());
     });
   });
+
+  it('does not use default filters when includeDefaults is false', () => {
+    const state = buildStateFromHparamsState(
+      buildHparamsState(
+        buildSpecs('foo', {
+          metric: {
+            specs: [buildMetricSpec({tag: 'acc'})],
+            defaultFilters: new Map([
+              [
+                'acc',
+                buildIntervalFilter({
+                  filterLowerValue: 0,
+                  filterUpperValue: 1,
+                }),
+              ],
+            ]),
+          },
+        }),
+        {}
+      )
+    );
+
+    expect(
+      selectors.getMetricFilterMapFromExperimentIds(['foo'], false)(state)
+    ).toEqual(new Map());
+  });
 });