Skip to content

Commit da812d2

Browse files
authored
Hparams: Allow users to load more Hparams if they are not all included in default. (#6780)
In #6777 we limited the number of hparams we load by default. Now we add the option for the user to load all hparams in spite of the performance implications. OSS Light Mode: ![image](https://github.com/tensorflow/tensorboard/assets/17152369/891856ac-bbf7-4b8b-be94-b776fe4bbe1e) OSS Dark Mode: ![image](https://github.com/tensorflow/tensorboard/assets/17152369/7f6fff68-73c2-4321-8912-8deb651be83d) Internal Light Mode: ![image](https://github.com/tensorflow/tensorboard/assets/17152369/b0e251c2-6b07-45e6-948d-bce24923d75a) Internal Dark Mode: ![image](https://github.com/tensorflow/tensorboard/assets/17152369/d8959cd1-d12c-401e-9a75-45eae5fa4551)
1 parent 571a6a6 commit da812d2

20 files changed

+132
-45
lines changed

tensorboard/webapp/hparams/_redux/hparams_actions.ts

+4
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,7 @@ export const dashboardHparamColumnOrderChanged = createAction(
7171
'[Hparams] Dashboard Hparam Column Order Changed',
7272
props<ReorderColumnEvent>()
7373
);
74+
75+
export const loadAllDashboardHparams = createAction(
76+
'[Hparams] Load all Hparams'
77+
);

tensorboard/webapp/hparams/_redux/hparams_effects.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,11 @@ export class HparamsEffects {
6262

6363
private readonly loadHparamsOnReload$: Observable<string[]> =
6464
this.actions$.pipe(
65-
ofType(coreActions.reload, coreActions.manualReload),
65+
ofType(
66+
coreActions.reload,
67+
coreActions.manualReload,
68+
hparamsActions.loadAllDashboardHparams
69+
),
6670
withLatestFrom(this.store.select(getExperimentIdsFromRoute)),
6771
filter(([, experimentIds]) => Boolean(experimentIds)),
6872
map(([, experimentIds]) => experimentIds as string[])

tensorboard/webapp/hparams/_redux/hparams_effects_test.ts

+30-35
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ describe('hparams effects', () => {
101101
dataSource.fetchSessionGroups.and.returnValue(of(mockSessionGroups));
102102
});
103103

104+
afterEach(() => {
105+
store?.resetSelectors();
106+
});
107+
104108
describe('loadHparamsData$', () => {
105109
beforeEach(() => {
106110
effects.loadHparamsData$.subscribe((action) => {
@@ -162,41 +166,32 @@ describe('hparams effects', () => {
162166
]);
163167
});
164168

165-
it('fetches data on reload', () => {
166-
action.next(coreActions.reload());
167-
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
168-
['expFromRoute'],
169-
1111
170-
);
171-
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
172-
['expFromRoute'],
173-
[buildHparamSpec({name: 'h1'})]
174-
);
175-
expect(actualActions).toEqual([
176-
hparamsActions.hparamsFetchSessionGroupsSucceeded({
177-
hparamSpecs: mockHparamSpecs,
178-
sessionGroups: mockSessionGroups,
179-
}),
180-
]);
181-
});
182-
183-
it('fetches data on manualReload', () => {
184-
action.next(coreActions.manualReload());
185-
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
186-
['expFromRoute'],
187-
1111
188-
);
189-
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
190-
['expFromRoute'],
191-
[buildHparamSpec({name: 'h1'})]
192-
);
193-
expect(actualActions).toEqual([
194-
hparamsActions.hparamsFetchSessionGroupsSucceeded({
195-
hparamSpecs: mockHparamSpecs,
196-
sessionGroups: mockSessionGroups,
197-
}),
198-
]);
199-
});
169+
for (const {actionName, actionInstance} of [
170+
{actionName: 'reload', actionInstance: coreActions.reload()},
171+
{actionName: 'manualReload', actionInstance: coreActions.manualReload()},
172+
{
173+
actionName: 'loadAllDashboardHparams',
174+
actionInstance: hparamsActions.loadAllDashboardHparams(),
175+
},
176+
]) {
177+
it(`fetches data on ${actionName}`, () => {
178+
action.next(actionInstance);
179+
expect(dataSource.fetchExperimentInfo).toHaveBeenCalledWith(
180+
['expFromRoute'],
181+
1111
182+
);
183+
expect(dataSource.fetchSessionGroups).toHaveBeenCalledWith(
184+
['expFromRoute'],
185+
[buildHparamSpec({name: 'h1'})]
186+
);
187+
expect(actualActions).toEqual([
188+
hparamsActions.hparamsFetchSessionGroupsSucceeded({
189+
hparamSpecs: mockHparamSpecs,
190+
sessionGroups: mockSessionGroups,
191+
}),
192+
]);
193+
});
194+
}
200195

201196
it('does not attempt to load hparams when experiment ids are null', () => {
202197
store.overrideSelector(selectors.getExperimentIdsFromRoute, null);

tensorboard/webapp/hparams/_redux/hparams_reducers.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,13 @@ const reducer: ActionReducer<HparamsState, Action> = createReducer(
166166
dashboardDisplayedHparamColumns: newColumns,
167167
};
168168
}
169-
)
169+
),
170+
on(actions.loadAllDashboardHparams, (state) => {
171+
return {
172+
...state,
173+
numDashboardHparamsToLoad: 0, // All.
174+
};
175+
})
170176
);
171177

172178
export function reducers(state: HparamsState | undefined, action: Action) {

tensorboard/webapp/hparams/_redux/hparams_reducers_test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -712,4 +712,16 @@ describe('hparams/_redux/hparams_reducers_test', () => {
712712
]);
713713
});
714714
});
715+
716+
describe('loadAllDashboardHparams', () => {
717+
it('sets numDashboardHparamsToLoad to 0', () => {
718+
const state = buildHparamsState({
719+
numDashboardHparamsToLoad: 1000,
720+
});
721+
722+
const state2 = reducers(state, actions.loadAllDashboardHparams());
723+
724+
expect(state2.numDashboardHparamsToLoad).toEqual(0);
725+
});
726+
});
715727
});

tensorboard/webapp/hparams/_redux/hparams_selectors_test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ limitations under the License.
1515

1616
import {ColumnHeaderType} from '../../widgets/data_table/types';
1717
import {DomainType} from '../types';
18-
import {State} from './types';
1918
import * as selectors from './hparams_selectors';
2019
import {
2120
buildHparamSpec,

tensorboard/webapp/metrics/views/card_renderer/scalar_card_component.ng.html

+1
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@
211211
(removeColumn)="removeColumn.emit($event)"
212212
(hideColumn)="hideColumn.emit($event)"
213213
(addFilter)="addFilter.emit($event)"
214+
(loadAllColumns)="loadAllColumns.emit()"
214215
>
215216
</scalar-card-data-table>
216217
</div>

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

+1
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ export class ScalarCardComponent<Downloader> {
132132
@Output() addColumn = new EventEmitter<AddColumnEvent>();
133133
@Output() removeColumn = new EventEmitter<HeaderToggleInfo>();
134134
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
135+
@Output() loadAllColumns = new EventEmitter<null>();
135136

136137
@Output() onLineChartZoom = new EventEmitter<Extent | null>();
137138
@Output() onCardStateChanged = new EventEmitter<Partial<CardState>>();

tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts

+5
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ function areSeriesEqual(
211211
(addColumn)="onAddColumn($event)"
212212
(removeColumn)="onRemoveColumn($event)"
213213
(addFilter)="addHparamFilter($event)"
214+
(loadAllColumns)="loadAllColumns()"
214215
></scalar-card-component>
215216
`,
216217
styles: [
@@ -763,4 +764,8 @@ export class ScalarCardContainer implements CardRenderer, OnInit, OnDestroy {
763764
})
764765
);
765766
}
767+
768+
loadAllColumns() {
769+
this.store.dispatch(hparamsActions.loadAllDashboardHparams());
770+
}
766771
}

tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
(addColumn)="addColumn.emit($event)"
2828
(removeColumn)="onRemoveColumn($event)"
2929
(addFilter)="addFilter.emit($event)"
30+
(loadAllColumns)="loadAllColumns.emit()"
3031
>
3132
<ng-container header>
3233
<ng-container *ngFor="let header of getHeaders()">

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

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export class ScalarCardDataTable {
7070
@Output() addColumn = new EventEmitter<AddColumnEvent>();
7171
@Output() removeColumn = new EventEmitter<HeaderToggleInfo>();
7272
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
73+
@Output() loadAllColumns = new EventEmitter<null>();
7374

7475
ColumnHeaderType = ColumnHeaderType;
7576

tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
(addColumn)="addColumn.emit($event)"
3939
(removeColumn)="removeColumn.emit($event)"
4040
(addFilter)="addFilter.emit($event)"
41+
(loadAllColumns)="loadAllColumns.emit()"
4142
>
4243
<ng-container header>
4344
<ng-container *ngFor="let header of getHeaders()">

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

+1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export class RunsDataTable {
6565
@Output() removeColumn = new EventEmitter<ColumnHeader>();
6666
@Output() onSelectionDblClick = new EventEmitter<string>();
6767
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
68+
@Output() loadAllColumns = new EventEmitter<null>();
6869

6970
// These columns must be stored and reused to stop needless re-rendering of
7071
// the content and headers in these columns. This has been known to cause

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

+5
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ const getRunsLoading = createSelector<
115115
(addColumn)="addColumn($event)"
116116
(removeColumn)="removeColumn($event)"
117117
(addFilter)="addHparamFilter($event)"
118+
(loadAllColumns)="loadAllColumns()"
118119
></runs-data-table>
119120
`,
120121
styles: [
@@ -361,6 +362,10 @@ export class RunsTableContainer implements OnInit, OnDestroy {
361362
})
362363
);
363364
}
365+
366+
loadAllColumns() {
367+
this.store.dispatch(hparamsActions.loadAllDashboardHparams());
368+
}
364369
}
365370

366371
export const TEST_ONLY = {

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

+8-3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@
2525

2626
<div class="column-load-info">
2727
<label>{{ numColumnsLoaded }} columns loaded.</label>
28-
<label *ngIf="hasMoreColumnsToLoad">
29-
Warning: There were too many columns to load all of them.
30-
</label>
28+
<div *ngIf="hasMoreColumnsToLoad" class="load-more-columns">
29+
<label>
30+
Warning: There were too many columns to load all of them efficiently.
31+
</label>
32+
<button mat-stroked-button (click)="loadAllColumnsClicked()">
33+
Load all anyway
34+
</button>
35+
</div>
3136
</div>
3237

3338
<div #columnList class="column-list">

tensorboard/webapp/widgets/data_table/column_selector_component.scss

+14
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,20 @@ limitations under the License.
4848
margin-bottom: 12px;
4949
}
5050

51+
.load-more-columns {
52+
color: mat.get-color-from-palette($tb-warn, 600);
53+
display: flex;
54+
flex-direction: column;
55+
margin-top: 6px;
56+
57+
button {
58+
color: inherit;
59+
font-size: inherit;
60+
height: 24px;
61+
margin-top: 8px;
62+
}
63+
}
64+
5165
.column-list {
5266
display: flex;
5367
flex-direction: column;

tensorboard/webapp/widgets/data_table/column_selector_component.ts

+5
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
3838
@Input() numColumnsLoaded!: number;
3939
@Input() hasMoreColumnsToLoad!: boolean;
4040
@Output() columnSelected = new EventEmitter<ColumnHeader>();
41+
@Output() loadAllColumns = new EventEmitter<null>();
4142

4243
@ViewChild('search')
4344
private readonly searchField!: ElementRef;
@@ -124,6 +125,10 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
124125
this.columnSelected.emit(header);
125126
}
126127

128+
loadAllColumnsClicked() {
129+
this.loadAllColumns.emit();
130+
}
131+
127132
activate() {
128133
this.isActive = true;
129134
}

tensorboard/webapp/widgets/data_table/column_selector_test.ts

+29-4
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ describe('column selector', () => {
4141
return fixture.debugElement.query(By.css('button.selected'));
4242
}
4343

44+
function getLoadAllColumnsButton() {
45+
return fixture.debugElement.query(By.css('.column-load-info button'));
46+
}
47+
4448
beforeEach(async () => {
4549
await TestBed.configureTestingModule({
4650
declarations: [ColumnSelectorComponent],
@@ -217,13 +221,34 @@ describe('column selector', () => {
217221
}));
218222

219223
it('renders too many columns warning', fakeAsync(() => {
220-
fixture.componentInstance.numColumnsLoaded = 100;
221224
fixture.componentInstance.hasMoreColumnsToLoad = true;
222225
fixture.detectChanges();
223226

224-
const numColumns = fixture.debugElement.query(By.css('.column-load-info'));
225-
expect(numColumns.nativeElement.textContent).toContain(
226-
'Warning: There were too many columns to load all of them.'
227+
const numColumnsEl = fixture.debugElement.query(
228+
By.css('.column-load-info')
227229
);
230+
expect(numColumnsEl.nativeElement.textContent).toContain(
231+
'Warning: There were too many columns to load all of them efficiently.'
232+
);
233+
}));
234+
235+
it('does not render "load all" button by default', fakeAsync(() => {
236+
const loadAllButton = getLoadAllColumnsButton();
237+
expect(loadAllButton).toBeFalsy();
238+
}));
239+
240+
it('renders "load all" button and responds to click', fakeAsync(() => {
241+
let loadAllColumnsClicked = false;
242+
fixture.componentInstance.loadAllColumns.subscribe(() => {
243+
loadAllColumnsClicked = true;
244+
});
245+
fixture.componentInstance.hasMoreColumnsToLoad = true;
246+
fixture.detectChanges();
247+
248+
const loadAllButton = getLoadAllColumnsButton();
249+
loadAllButton.nativeElement.click();
250+
flush();
251+
252+
expect(loadAllColumnsClicked).toBeTrue();
228253
}));
229254
});

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

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
[numColumnsLoaded]="numColumnsLoaded"
8484
[hasMoreColumnsToLoad]="hasMoreColumnsToLoad"
8585
(columnSelected)="onColumnAdded($event)"
86+
(loadAllColumns)="loadAllColumns.emit()"
8687
></tb-data-table-column-selector-component>
8788
</custom-modal>
8889

tensorboard/webapp/widgets/data_table/data_table_component.ts

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export class DataTableComponent implements OnDestroy, AfterContentInit {
8383
@Output() removeColumn = new EventEmitter<ColumnHeader>();
8484
@Output() addColumn = new EventEmitter<AddColumnEvent>();
8585
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
86+
@Output() loadAllColumns = new EventEmitter<null>();
8687

8788
@ViewChild('columnSelectorModal', {static: false})
8889
private readonly columnSelectorModal!: CustomModalComponent;

0 commit comments

Comments
 (0)