Skip to content

Commit 67b6aa0

Browse files
JamesHollyerJamesHollyer
committed
HParam UI: update ColumnHeader type (tensorflow#6346)
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. 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) N/A POC works 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.
1 parent 5ea918e commit 67b6aa0

File tree

7 files changed

+252
-319
lines changed

7 files changed

+252
-319
lines changed

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ export class ScalarCardComponent<Downloader> {
122122
@ViewChild(LineChartComponent)
123123
lineChart?: LineChartComponent;
124124
sortingInfo: SortingInfo = {
125-
header: ColumnHeaderType.RUN,
125+
header: ColumnHeaderType.RUN, //This is no longer used but the type needs it or it will break sync. TODO(jameshollyer): remove this once internal code all uses name.
126+
name: 'run',
126127
order: SortingOrder.ASCENDING,
127128
};
128129

‎tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts‎

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -131,60 +131,60 @@ export class ScalarCardDataTable {
131131
}
132132
const selectedStepData: SelectedStepRunData = {
133133
id: datum.id,
134+
color: metadata.color,
134135
};
135-
selectedStepData.COLOR = metadata.color;
136136
for (const header of this.columnHeaders) {
137137
switch (header.type) {
138138
case ColumnHeaderType.RUN:
139139
let alias = '';
140140
if (metadata.alias) {
141141
alias = `${metadata.alias.aliasNumber} ${metadata.alias.aliasText}/`;
142142
}
143-
selectedStepData.RUN = `${alias}${metadata.displayName}`;
143+
selectedStepData[header.name] = `${alias}${metadata.displayName}`;
144144
continue;
145145
case ColumnHeaderType.STEP:
146-
selectedStepData.STEP = closestStartPoint.step;
146+
selectedStepData[header.name] = closestStartPoint.step;
147147
continue;
148148
case ColumnHeaderType.VALUE:
149-
selectedStepData.VALUE = closestStartPoint.value;
149+
selectedStepData[header.name] = closestStartPoint.value;
150150
continue;
151151
case ColumnHeaderType.RELATIVE_TIME:
152-
selectedStepData.RELATIVE_TIME =
152+
selectedStepData[header.name] =
153153
closestStartPoint.relativeTimeInMs;
154154
continue;
155155
case ColumnHeaderType.SMOOTHED:
156-
selectedStepData.SMOOTHED = closestStartPoint.y;
156+
selectedStepData[header.name] = closestStartPoint.y;
157157
continue;
158158
case ColumnHeaderType.VALUE_CHANGE:
159159
if (!closestEndPoint) {
160160
continue;
161161
}
162-
selectedStepData.VALUE_CHANGE =
162+
selectedStepData[header.name] =
163163
closestEndPoint.y - closestStartPoint.y;
164164
continue;
165165
case ColumnHeaderType.START_STEP:
166-
selectedStepData.START_STEP = closestStartPoint.step;
166+
selectedStepData[header.name] = closestStartPoint.step;
167167
continue;
168168
case ColumnHeaderType.END_STEP:
169169
if (!closestEndPoint) {
170170
continue;
171171
}
172-
selectedStepData.END_STEP = closestEndPoint.step;
172+
selectedStepData[header.name] = closestEndPoint.step;
173173
continue;
174174
case ColumnHeaderType.START_VALUE:
175-
selectedStepData.START_VALUE = closestStartPoint.y;
175+
selectedStepData[header.name] = closestStartPoint.y;
176176
continue;
177177
case ColumnHeaderType.END_VALUE:
178178
if (!closestEndPoint) {
179179
continue;
180180
}
181-
selectedStepData.END_VALUE = closestEndPoint.y;
181+
selectedStepData[header.name] = closestEndPoint.y;
182182
continue;
183183
case ColumnHeaderType.MIN_VALUE:
184184
if (!closestEndPointIndex) {
185185
continue;
186186
}
187-
selectedStepData.MIN_VALUE = this.getMinPointInRange(
187+
selectedStepData[header.name] = this.getMinPointInRange(
188188
datum.points,
189189
closestStartPointIndex,
190190
closestEndPointIndex
@@ -194,7 +194,7 @@ export class ScalarCardDataTable {
194194
if (!closestEndPointIndex) {
195195
continue;
196196
}
197-
selectedStepData.MAX_VALUE = this.getMaxPointInRange(
197+
selectedStepData[header.name] = this.getMaxPointInRange(
198198
datum.points,
199199
closestStartPointIndex,
200200
closestEndPointIndex
@@ -204,14 +204,14 @@ export class ScalarCardDataTable {
204204
if (!closestEndPoint) {
205205
continue;
206206
}
207-
selectedStepData.PERCENTAGE_CHANGE =
207+
selectedStepData[header.name] =
208208
(closestEndPoint.y - closestStartPoint.y) / closestStartPoint.y;
209209
continue;
210210
case ColumnHeaderType.STEP_AT_MAX:
211211
if (!closestEndPointIndex) {
212212
continue;
213213
}
214-
selectedStepData.STEP_AT_MAX = this.getMaxPointInRange(
214+
selectedStepData[header.name] = this.getMaxPointInRange(
215215
datum.points,
216216
closestStartPointIndex,
217217
closestEndPointIndex
@@ -221,7 +221,7 @@ export class ScalarCardDataTable {
221221
if (!closestEndPointIndex) {
222222
continue;
223223
}
224-
selectedStepData.STEP_AT_MIN = this.getMinPointInRange(
224+
selectedStepData[header.name] = this.getMinPointInRange(
225225
datum.points,
226226
closestStartPointIndex,
227227
closestEndPointIndex
@@ -231,7 +231,7 @@ export class ScalarCardDataTable {
231231
if (!closestEndPointIndex) {
232232
continue;
233233
}
234-
selectedStepData.MEAN = this.getMean(
234+
selectedStepData[header.name] = this.getMean(
235235
datum.points,
236236
closestStartPointIndex,
237237
closestEndPointIndex
@@ -241,7 +241,7 @@ export class ScalarCardDataTable {
241241
if (!closestEndPoint) {
242242
continue;
243243
}
244-
selectedStepData.RAW_CHANGE =
244+
selectedStepData[header.name] =
245245
closestEndPoint.value - closestStartPoint.value;
246246
continue;
247247
default:
@@ -250,35 +250,39 @@ export class ScalarCardDataTable {
250250
}
251251
return selectedStepData;
252252
});
253-
dataTableData.sort(
254-
(point1: SelectedStepRunData, point2: SelectedStepRunData) => {
255-
const p1 = this.getSortableValue(point1, this.sortingInfo.header);
256-
const p2 = this.getSortableValue(point2, this.sortingInfo.header);
257-
if (p1 < p2) {
258-
return this.sortingInfo.order === SortingOrder.ASCENDING ? -1 : 1;
259-
}
260-
if (p1 > p2) {
261-
return this.sortingInfo.order === SortingOrder.ASCENDING ? 1 : -1;
262-
}
263-
264-
return 0;
265-
}
253+
const sortingHeader = this.columnHeaders.find(
254+
(header) => header.name === this.sortingInfo.name
266255
);
256+
if (sortingHeader !== undefined) {
257+
dataTableData.sort(
258+
(point1: SelectedStepRunData, point2: SelectedStepRunData) => {
259+
if (!sortingHeader) {
260+
return 0;
261+
}
262+
const p1 = this.getSortableValue(point1, sortingHeader);
263+
const p2 = this.getSortableValue(point2, sortingHeader);
264+
if (p1 < p2) {
265+
return this.sortingInfo.order === SortingOrder.ASCENDING ? -1 : 1;
266+
}
267+
if (p1 > p2) {
268+
return this.sortingInfo.order === SortingOrder.ASCENDING ? 1 : -1;
269+
}
270+
271+
return 0;
272+
}
273+
);
274+
}
267275
return dataTableData;
268276
}
269277

270-
private getSortableValue(
271-
point: SelectedStepRunData,
272-
header: ColumnHeaderType
273-
) {
274-
switch (header) {
275-
// The value shown in the "RUN" column is a string concatenation of Alias Id + Alias + Run Name
276-
// but we would actually prefer to sort by just the run name.
277-
case ColumnHeaderType.RUN:
278-
return makeValueSortable(this.chartMetadataMap[point.id].displayName);
279-
default:
280-
return makeValueSortable(point[header]);
278+
private getSortableValue(point: SelectedStepRunData, header: ColumnHeader) {
279+
// The value shown in the "RUN" column is a string concatenation of Alias Id + Alias + Run Name
280+
// but we would actually prefer to sort by just the run name.
281+
if (header.type === ColumnHeaderType.RUN) {
282+
return makeValueSortable(this.chartMetadataMap[point.id].displayName);
281283
}
284+
285+
return makeValueSortable(point[header.name]);
282286
}
283287

284288
orderColumns(headers: ColumnHeader[]) {

0 commit comments

Comments
 (0)