Skip to content

Commit

Permalink
HParam UI: update ColumnHeader type (#6346)
Browse files Browse the repository at this point in the history
## Motivation for features / changes
The goals is to allow for dynamic HParam columns in the data table. This
means that we cannot have an enum like ColumnHeaderType be the unique
identifier for the column headers because we cannot encapsulate all
possible hparams into an enum.

## Technical description of changes
This change adds a name and displayName field to the ColumnHeader
interface. The goal here is to make name be the unique identifier for
columns. However, this PR does not use these fields at all. It simply
adds them. THERE IS NO FUNCTIONAL CHANGE in this PR.

Upcoming changes will switch the code over to start using the name as
the unique identifier and display the displayName.

(For POC to see how this change fits in googlers see cl/526767686)

## Screenshots of UI changes (or N/A)
N/A

## Detailed steps to verify changes work correctly (as executed by you)
POC works

## Alternate designs / implementations considered (or N/A)
There were many alternatives considered. In fact I do not believe this
is going to be the final structure.

One option is to remove the type and add a category field
export interface ColumnHeader {
  name: string;
  displayName: string;
  Category: ColumnCategory(STATIC, HPARAM, METRIC)
  enabled: boolean;
}

However, deciding how to calculate the value based on hardcoded name
strings does not seem ideal.

Another option is to have a hierarchy. I expect it to look like this:
Interface ColumnHeaderBase {
  name: string;
  displayName: string;
  type: ColumnHeaderType;
  enabled: boolean;
}

export interface StaticColumnHeader extends ColumnHeaderBase{
	category: “STATIC”
}

export interface HParamColumnHeader extends ColumnHeaderBase{
	category: “HParam”
        source: enum(data, config,....);
        differs:boolean;
}

ColumnHeader = StaticColumnHeader | HParamColumnHeader

This may be the final form that we go with however, we are not sure the
source and differs attributes need to go here or somewhere else yet. All
code which works with the type defined in this PR will work with this
proposal as well.

Lastly, there is the option which is the same as the hierarchy option
above but with StaticColumnHeader being the only interface with a "type"
attribute. This is a valid possibility but, I have decided that it just
adds unnecessary complexity by requiring a type check in many more
places.
  • Loading branch information
JamesHollyer authored May 2, 2023
1 parent 7e751fc commit 914f662
Show file tree
Hide file tree
Showing 8 changed files with 1,684 additions and 287 deletions.
128 changes: 109 additions & 19 deletions tensorboard/webapp/metrics/store/metrics_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,26 +269,116 @@ const {initialState, reducers: namespaceContextedReducer} =
stepSelectorEnabled: false,
rangeSelectionEnabled: false,
singleSelectionHeaders: [
{type: ColumnHeaderType.RUN, enabled: true},
{type: ColumnHeaderType.SMOOTHED, enabled: true},
{type: ColumnHeaderType.VALUE, enabled: true},
{type: ColumnHeaderType.STEP, enabled: true},
{type: ColumnHeaderType.RELATIVE_TIME, enabled: true},
{
type: ColumnHeaderType.RUN,
name: 'run',
displayName: 'Run',
enabled: true,
},
{
type: ColumnHeaderType.SMOOTHED,
name: 'smoothed',
displayName: 'Smoothed',
enabled: true,
},
{
type: ColumnHeaderType.VALUE,
name: 'value',
displayName: 'Value',
enabled: true,
},
{
type: ColumnHeaderType.STEP,
name: 'step',
displayName: 'Step',
enabled: true,
},
{
type: ColumnHeaderType.RELATIVE_TIME,
name: 'relative',
displayName: 'Relative',
enabled: true,
},
],
rangeSelectionHeaders: [
{type: ColumnHeaderType.RUN, enabled: true},
{type: ColumnHeaderType.MIN_VALUE, enabled: true},
{type: ColumnHeaderType.MAX_VALUE, enabled: true},
{type: ColumnHeaderType.START_VALUE, enabled: true},
{type: ColumnHeaderType.END_VALUE, enabled: true},
{type: ColumnHeaderType.VALUE_CHANGE, enabled: true},
{type: ColumnHeaderType.PERCENTAGE_CHANGE, enabled: true},
{type: ColumnHeaderType.START_STEP, enabled: true},
{type: ColumnHeaderType.END_STEP, enabled: true},
{type: ColumnHeaderType.STEP_AT_MAX, enabled: false},
{type: ColumnHeaderType.STEP_AT_MIN, enabled: false},
{type: ColumnHeaderType.MEAN, enabled: false},
{type: ColumnHeaderType.RAW_CHANGE, enabled: false},
{
type: ColumnHeaderType.RUN,
name: 'run',
displayName: 'Run',
enabled: true,
},
{
type: ColumnHeaderType.MIN_VALUE,
name: 'min',
displayName: 'Min',
enabled: true,
},
{
type: ColumnHeaderType.MAX_VALUE,
name: 'max',
displayName: 'Max',
enabled: true,
},
{
type: ColumnHeaderType.START_VALUE,
name: 'start',
displayName: 'Start Value',
enabled: true,
},
{
type: ColumnHeaderType.END_VALUE,
name: 'end',
displayName: 'End Value',
enabled: true,
},
{
type: ColumnHeaderType.VALUE_CHANGE,
name: 'valueChange',
displayName: 'Value',
enabled: true,
},
{
type: ColumnHeaderType.PERCENTAGE_CHANGE,
name: 'percentageChange',
displayName: '%',
enabled: true,
},
{
type: ColumnHeaderType.START_STEP,
name: 'startStep',
displayName: 'Start Step',
enabled: true,
},
{
type: ColumnHeaderType.END_STEP,
name: 'endStep',
displayName: 'End Step',
enabled: true,
},
{
type: ColumnHeaderType.STEP_AT_MAX,
name: 'stepAtMax',
displayName: 'Step At Max',
enabled: false,
},
{
type: ColumnHeaderType.STEP_AT_MIN,
name: 'stepAtMin',
displayName: 'Step At Min',
enabled: false,
},
{
type: ColumnHeaderType.MEAN,
name: 'mean',
displayName: 'Mean',
enabled: false,
},
{
type: ColumnHeaderType.RAW_CHANGE,
name: 'rawChange',
displayName: 'Raw',
enabled: false,
},
],
filteredPluginTypes: new Set(),
stepMinMax: {
Expand Down Expand Up @@ -1308,7 +1398,7 @@ const reducer = createReducer(
);

newHeaders[newToggledHeaderIndex] = {
type: newHeaders[newToggledHeaderIndex].type,
...newHeaders[newToggledHeaderIndex],
enabled: !newHeaders[newToggledHeaderIndex].enabled,
};

Expand Down
Loading

0 comments on commit 914f662

Please sign in to comment.