Skip to content

Commit 388ddf5

Browse files
committed
Fix the issue with coloring when trying to add more experiments.
There was a problem when user tried to 1) enable coloring by experiment name, 2) trying to add more experiments. It stemmed from not implementing the serialization and deserialization of REGEX_BY_EXP in the query pararms of the route. To followup the tensorflow#6846 and tensorflow#6847 I have added the 'regex_exp:' as a query parameter for coloring by experiment name.
1 parent ae7d0b9 commit 388ddf5

File tree

4 files changed

+71
-1
lines changed

4 files changed

+71
-1
lines changed

tensorboard/webapp/routes/dashboard_deeplink_provider.ts

+10
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
import {featureFlagsToSerializableQueryParams} from './feature_flag_serializer';
4343

4444
const COLOR_GROUP_REGEX_VALUE_PREFIX = 'regex:';
45+
const COLOR_GROUP_REGEX_EXP_VALUE_PREFIX = 'regex_exp:';
4546

4647
/**
4748
* Provides deeplinking for the core dashboards page.
@@ -135,6 +136,9 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
135136
case GroupByKey.REGEX:
136137
value = `${COLOR_GROUP_REGEX_VALUE_PREFIX}${groupBy.regexString}`;
137138
break;
139+
case GroupByKey.REGEX_BY_EXP:
140+
value = `${COLOR_GROUP_REGEX_EXP_VALUE_PREFIX}${groupBy.regexString}`;
141+
break;
138142
default:
139143
throw new RangeError(`Serialization not implemented`);
140144
}
@@ -196,6 +200,12 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
196200
);
197201
groupBy = {key: GroupByKey.REGEX, regexString};
198202
}
203+
if (value.startsWith(COLOR_GROUP_REGEX_EXP_VALUE_PREFIX)) {
204+
const regexString = value.slice(
205+
COLOR_GROUP_REGEX_EXP_VALUE_PREFIX.length
206+
);
207+
groupBy = {key: GroupByKey.REGEX_BY_EXP, regexString};
208+
}
199209
break;
200210
}
201211
case TAG_FILTER_KEY:

tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts

+33
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,16 @@ describe('core deeplink provider', () => {
123123
assert('run', {key: GroupByKey.RUN});
124124
assert('regex:', {key: GroupByKey.REGEX, regexString: ''});
125125
assert('regex:hello', {key: GroupByKey.REGEX, regexString: 'hello'});
126+
assert('regex_exp:', {key: GroupByKey.REGEX_BY_EXP, regexString: ''});
127+
assert('regex_exp:world', {
128+
key: GroupByKey.REGEX_BY_EXP,
129+
regexString: 'world',
130+
});
126131
assert('', null);
127132
assert('regex', null);
128133
assert('runs', null);
129134
assert('experiments', null);
135+
assert('regex_exp', null);
130136
});
131137
});
132138

@@ -434,6 +440,15 @@ describe('core deeplink provider', () => {
434440
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
435441
[{key: 'runColorGroup', value: 'regex:hello:world'}]
436442
);
443+
444+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
445+
key: GroupByKey.REGEX_BY_EXP,
446+
regexString: 'exp_name',
447+
});
448+
store.refreshState();
449+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
450+
[{key: 'runColorGroup', value: 'regex_exp:exp_name'}]
451+
);
437452
});
438453

439454
it('serializes interesting regex strings', () => {
@@ -454,6 +469,24 @@ describe('core deeplink provider', () => {
454469
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
455470
[{key: 'runColorGroup', value: 'regex:hello/(world):goodbye'}]
456471
);
472+
473+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
474+
key: GroupByKey.REGEX_BY_EXP,
475+
regexString: '',
476+
});
477+
store.refreshState();
478+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
479+
[{key: 'runColorGroup', value: 'regex_exp:'}]
480+
);
481+
482+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
483+
key: GroupByKey.REGEX_BY_EXP,
484+
regexString: 'one|two',
485+
});
486+
store.refreshState();
487+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
488+
[{key: 'runColorGroup', value: 'regex_exp:one|two'}]
489+
);
457490
});
458491
});
459492

tensorboard/webapp/runs/store/utils.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,11 @@ export function groupRuns(
7777
break;
7878

7979
case GroupByKey.REGEX_BY_EXP:
80-
if (!groupBy.regexString || !expNameByExpId) {
80+
if (
81+
!groupBy.regexString ||
82+
!expNameByExpId ||
83+
Object.keys(expNameByExpId).length === 0
84+
) {
8185
break;
8286
}
8387

tensorboard/webapp/runs/store/utils_test.ts

+23
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,29 @@ describe('run store utils test', () => {
292292
});
293293
});
294294

295+
it('does not group if no experiment names are provided', () => {
296+
const actual = groupRuns(
297+
{key: GroupByKey.REGEX_BY_EXP, regexString: 'foo\\d+)bar'},
298+
[
299+
buildRun({id: 'eid1/alpha', name: 'foo1bar1'}),
300+
buildRun({id: 'eid1/beta', name: 'foo1bar2'}),
301+
buildRun({id: 'eid2/beta', name: 'foo2bar1'}),
302+
buildRun({id: 'eid2/gamma', name: 'gamma'}),
303+
],
304+
{
305+
'eid1/alpha': 'eid1',
306+
'eid1/beta': 'eid1',
307+
'eid2/beta': 'eid2',
308+
'eid2/gamma': 'eid2',
309+
},
310+
{}
311+
);
312+
expect(actual).toEqual({
313+
matches: {},
314+
nonMatches: [],
315+
});
316+
});
317+
295318
it('groups run by regex without capture group', () => {
296319
const actual = groupRuns(
297320
{key: GroupByKey.REGEX_BY_EXP, regexString: 'foo'},

0 commit comments

Comments
 (0)