Skip to content

Commit 6268a35

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 6268a35

File tree

4 files changed

+74
-1
lines changed

4 files changed

+74
-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_BY_EXP_VALUE_PREFIX = 'regex_by_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_BY_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_BY_EXP_VALUE_PREFIX)) {
204+
const regexString = value.slice(
205+
COLOR_GROUP_REGEX_BY_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

+36
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,19 @@ 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_by_exp:', {
127+
key: GroupByKey.REGEX_BY_EXP,
128+
regexString: '',
129+
});
130+
assert('regex_by_exp:world', {
131+
key: GroupByKey.REGEX_BY_EXP,
132+
regexString: 'world',
133+
});
126134
assert('', null);
127135
assert('regex', null);
128136
assert('runs', null);
129137
assert('experiments', null);
138+
assert('regex_by_exp', null);
130139
});
131140
});
132141

@@ -434,6 +443,15 @@ describe('core deeplink provider', () => {
434443
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
435444
[{key: 'runColorGroup', value: 'regex:hello:world'}]
436445
);
446+
447+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
448+
key: GroupByKey.REGEX_BY_EXP,
449+
regexString: 'exp_name',
450+
});
451+
store.refreshState();
452+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
453+
[{key: 'runColorGroup', value: 'regex_by_exp:exp_name'}]
454+
);
437455
});
438456

439457
it('serializes interesting regex strings', () => {
@@ -454,6 +472,24 @@ describe('core deeplink provider', () => {
454472
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
455473
[{key: 'runColorGroup', value: 'regex:hello/(world):goodbye'}]
456474
);
475+
476+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
477+
key: GroupByKey.REGEX_BY_EXP,
478+
regexString: '',
479+
});
480+
store.refreshState();
481+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
482+
[{key: 'runColorGroup', value: 'regex_by_exp:'}]
483+
);
484+
485+
store.overrideSelector(selectors.getRunUserSetGroupBy, {
486+
key: GroupByKey.REGEX_BY_EXP,
487+
regexString: 'one|two',
488+
});
489+
store.refreshState();
490+
expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
491+
[{key: 'runColorGroup', value: 'regex_by_exp:one|two'}]
492+
);
457493
});
458494
});
459495

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)