HParams: Allow runs in the time series dashboard to be filtered by hparam #6488

rileyajones · 2023-07-10T19:18:33Z

Motivation for features / changes

The previous runs table allowed the rows to be filtered by hparam. We would like to maintain this behavior.

Technical description of changes

The existing runs table had an UI for adding hparam filters but the components are all contained within the runs table component. Therefore I am breaking the UI affordances out into discrete components.

Screenshots of UI changes (or N/A)

The UI (Light Mode):

The UI (Dark Mode):

Interval Filters:

Filtered:

No Matches:

The option does not appear for non filterable columns:

Using The Feature

tensorboard/webapp/runs/store/runs_reducers.ts

rileyajones · 2023-07-11T21:05:27Z

tensorboard/webapp/runs/data_source/runs_data_source_types.ts

@@ -16,6 +16,9 @@ import {Injectable} from '@angular/core';
 import {Observable} from 'rxjs';
 import * as backendTypes from './runs_backend_types';

+import {DomainType} from '../../widgets/data_table/types';
+export {DomainType} from '../../widgets/data_table/types';


I don't care for reexporting this way but there are a bunch of other places which import his type then reexport it so it is non trivial to fix.

rileyajones · 2023-07-11T21:07:27Z

tensorboard/webapp/runs/data_source/BUILD

@@ -24,7 +25,7 @@ tf_ts_library(
    srcs = [
        "runs_backend_types.ts",
    ],
-    visibility = ["//visibility:private"],
+    visibility = ["//tensorboard/webapp/runs/data_source:__subpackages__", "//tensorboard/webapp/widgets/data_table:__subpackages__",],


This web of dependencies is pretty messy and I would love to clean it up, but I would rather not block the launch of this feature on this cleanup.

tensorboard/webapp/runs/views/runs_table/runs_data_table.ts

rileyajones · 2023-07-11T21:10:44Z

tensorboard/webapp/widgets/data_table/BUILD

+        "//tensorboard/webapp/angular:expect_angular_material_button",
+        "//tensorboard/webapp/angular:expect_angular_material_checkbox",


This will likely lead to issues when this is synced internally.

rileyajones · 2023-07-11T21:12:35Z

tensorboard/webapp/widgets/data_table/filter_dialogue_module.ts

@@ -0,0 +1,35 @@
+/* Copyright 2023 The TensorFlow Authors. All Rights Reserved.


It's quite possible that this should be it's own widget rather than being part of the data table widget. This approach seemed slightly easier but I'm quite open to feedback on this.

rileyajones · 2023-07-11T21:13:30Z

tensorboard/webapp/widgets/data_table/types.ts

@@ -53,6 +90,7 @@ export interface ColumnHeader {
  removable?: boolean;
  sortable?: boolean;
  movable?: boolean;
+  filterable?: boolean;


Technically we could use the existence of a value in the columnFilters map to determine this but I prefer this approach to keep things consistent.

rileyajones · 2023-07-11T21:14:36Z

tensorboard/webapp/widgets/data_table/types.ts

+export enum DomainType {
+  DISCRETE,
+  INTERVAL,
+}
+
+export type DiscreteFilterValues = string[] | number[] | boolean[];
+
+export type DiscreteFilterValue = DiscreteFilterValues[number];
+
+export interface DiscreteFilter {
+  type: DomainType.DISCRETE;
+  includeUndefined: boolean;
+  possibleValues: DiscreteFilterValues;
+  // Subset of `possibleValues`
+  filterValues: DiscreteFilterValues;
+}
+
+export interface DiscreteFilterChange {
+  hparamName: string;
+  includeUndefined: boolean;
+  filterValues: DiscreteFilterValues;
+}
+
+export interface IntervalFilter {
+  type: DomainType.INTERVAL;
+  includeUndefined: boolean;
+  minValue: number;
+  maxValue: number;
+  // Filter values have to be in between min and max values (inclusive).
+  filterLowerValue: number;
+  filterUpperValue: number;
+}


These have all been copied over from runs_data_source_types. I actually prefer moving these all to an even more independent location but would prefer to do that as a later cleanup.

rileyajones · 2023-07-11T21:15:19Z

tensorboard/webapp/widgets/data_table/types.ts

+export type DiscreteFilterValues = string[] | number[] | boolean[];
+
+export type DiscreteFilterValue = DiscreteFilterValues[number];


These two have been copied and renamed rather than moved. Once the old runs table is removed the old versions can be removed.

rileyajones · 2023-07-11T21:20:26Z

tensorboard/webapp/hparams/_types.ts

-export interface DiscreteFilter {
-  type: DomainType.DISCRETE;
-  includeUndefined: boolean;
-  possibleValues: DiscreteHparamValues;
-  // Subset of `possibleValues`
-  filterValues: DiscreteHparamValues;
-}
-
-export interface IntervalFilter {
-  type: DomainType.INTERVAL;
-  includeUndefined: boolean;
-  minValue: number;
-  maxValue: number;
-  // Filter values have to be in between min and max values (inclusive).
-  filterLowerValue: number;
-  filterUpperValue: number;
-}


These have been moved to the data table types because that is where the filtering is being done now.

## Motivation for features / changes We want to be able to filter the runs table by hparam. The existing logic for this is handled by a bunch of inner components found in the runs_table_component which makes it hard to share. In this PR I am replicating that UI with some enhancements in a standalone component which will later be used by the data_table_component. See #6488 for screenshots and a more complete description of what is being added ## Alternate designs / implementations considered (or N/A) I'm not sure that this component should be part of the data table but that is simplest place to put it for now.

…6553) ## Motivation for features / changes Now that the filter dialog is merged #6493 we need to add support for filtering by hparams. However, #6544 changed the way hparam values are read. This change adds a new property to state in which to store dashboard related hparam and metric filters. See #6488 for the WIP integration with the runs_table_container + tb_data_table -> common_selectors + hparam_selectors.

Merge branch 'master' into hparam-filters

## Motivation for features / changes We want to be able to filter the runs table by hparam. The existing logic for this is handled by a bunch of inner components found in the runs_table_component which makes it hard to share. In this PR I am replicating that UI with some enhancements in a standalone component which will later be used by the data_table_component. See tensorflow#6488 for screenshots and a more complete description of what is being added ## Alternate designs / implementations considered (or N/A) I'm not sure that this component should be part of the data table but that is simplest place to put it for now.

…ensorflow#6553) ## Motivation for features / changes Now that the filter dialog is merged tensorflow#6493 we need to add support for filtering by hparams. However, tensorflow#6544 changed the way hparam values are read. This change adds a new property to state in which to store dashboard related hparam and metric filters. See tensorflow#6488 for the WIP integration with the runs_table_container + tb_data_table -> common_selectors + hparam_selectors.

JamesHollyer · 2023-08-28T21:56:12Z

tensorboard/webapp/metrics/views/main_view/common_selectors.ts

  getDashboardHparamsAndMetricsSpecs,
  getDashboardHparamFilterMap,
+  getDashboardMetricsFilterMap,


Was this re ordered for a reason? I would rather the only change here be the addition of getDashboardDefaultHparamFilters

tensorboard/webapp/widgets/data_table/data_table_component.ng.html

tensorboard/webapp/widgets/data_table/data_table_component.ts

tensorboard/webapp/runs/data_source/runs_data_source.ts

tensorboard/webapp/runs/views/runs_table/runs_data_table.ts

tensorboard/webapp/runs/views/runs_table/runs_table_test.ts

JamesHollyer

Looks good after you fix that DomainType import in the runs_data_source file.

…6568) ## Motivation for features / changes `zip` and `forkJoin` work somewhat differently internally so these observables simply aren't emitting and thus break our sync. Googlers see cl/562877415 for the breakage and cl/cl/563214152 as proof that this works. #6566 #6488

rileyajones commented Jul 11, 2023

View reviewed changes

rileyajones force-pushed the hparam-filters branch 2 times, most recently from 79e2d4d to 3146158 Compare July 11, 2023 21:22

rileyajones commented Jul 11, 2023

View reviewed changes

rileyajones changed the title ~~[WIP] HParams: Allow runs in the time series dashboard to be filtered by hparam~~ HParams: Allow runs in the time series dashboard to be filtered by hparam Jul 11, 2023

rileyajones mentioned this pull request Jul 13, 2023

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

Merged

rileyajones force-pushed the hparam-filters branch from a4ad84b to 0cd4a00 Compare July 13, 2023 20:16

rileyajones mentioned this pull request Jul 13, 2023

HParams: Create filter dialog component #6493

Merged

rileyajones force-pushed the hparam-filters branch from 0cd4a00 to d5fd188 Compare August 21, 2023 22:46

rileyajones mentioned this pull request Aug 22, 2023

HParams: Add new state property to store hparam and metric filters #6553

Merged

rileyajones added 2 commits August 22, 2023 18:52

Integrate filter dialog with data table

f4e7334

add new hparams selectors to get default filters

ed5a78a

rileyajones force-pushed the hparam-filters branch from d5fd188 to ed5a78a Compare August 22, 2023 18:59

rileyajones added 2 commits August 22, 2023 20:43

update test to account for actions and selectors

83dedc8

Date: Thu Aug 24 22:20:31 2023 +0000

adb4252

Merge branch 'master' into hparam-filters

rileyajones force-pushed the hparam-filters branch from b35316a to adb4252 Compare August 25, 2023 00:22

Merge branch 'master' into hparam-filters

043935a

rileyajones marked this pull request as ready for review August 25, 2023 00:32

rileyajones requested a review from JamesHollyer August 25, 2023 00:32

JamesHollyer reviewed Aug 28, 2023

View reviewed changes

address pr comments

2cedf1a

rileyajones requested a review from JamesHollyer August 29, 2023 22:03

JamesHollyer approved these changes Aug 29, 2023

View reviewed changes

revert changes to runs_data_source

5eeaf06

rileyajones merged commit 3a70adc into tensorflow:master Aug 30, 2023
13 checks passed

rileyajones mentioned this pull request Sep 6, 2023

Bug Fix change run effects to use combine latest instead of forkjoin #6568

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HParams: Allow runs in the time series dashboard to be filtered by hparam #6488

HParams: Allow runs in the time series dashboard to be filtered by hparam #6488

rileyajones commented Jul 10, 2023 •

edited

Loading

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

rileyajones Jul 11, 2023

JamesHollyer Aug 28, 2023

JamesHollyer left a comment

		"//tensorboard/webapp/angular:expect_angular_material_button",
		"//tensorboard/webapp/angular:expect_angular_material_checkbox",

		@@ -0,0 +1,35 @@
		/* Copyright 2023 The TensorFlow Authors. All Rights Reserved.

		export type DiscreteFilterValues = string[] \| number[] \| boolean[];

		export type DiscreteFilterValue = DiscreteFilterValues[number];

HParams: Allow runs in the time series dashboard to be filtered by hparam #6488

HParams: Allow runs in the time series dashboard to be filtered by hparam #6488

Conversation

rileyajones commented Jul 10, 2023 • edited Loading

Motivation for features / changes

Technical description of changes

Screenshots of UI changes (or N/A)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesHollyer left a comment

Choose a reason for hiding this comment

rileyajones commented Jul 10, 2023 •

edited

Loading