Skip to content

Commit 70273bf

Browse files
authored
linked time: update scalar to "real" min max step (#5993)
* Motivation for features / changes The "minMax" input of scalar fob controller is actually the viewExtend/viewBox, not the min max step, while it's got used as the min max step. Visually the extend is larger than the min/max step and it is what we need for forward and reverse function. ``` A B (chart) C D |---|-------------|---| ``` axis size: A to D minMax: A to D line chart area: B to C min step: B max step C This wasn't noticed because we clip the time selection at the scalar card level to prevent the fob go out of the valid range (B to C) * Technical description of changes This PR adds another minMaxStep input and rename the old minMax to minMaxHorizentalViewExtend. (Note: skip the rename in test since that matters less) * Screenshots of UI changes Everything should be the same.
1 parent d8b7a9d commit 70273bf

File tree

3 files changed

+33
-62
lines changed

3 files changed

+33
-62
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@
231231
<scalar-card-fob-controller
232232
[timeSelection]="stepOrLinkedTimeSelection"
233233
[scale]="xScale"
234-
[minMax]="viewExtent.x"
234+
[minMaxHorizontalViewExtend]="viewExtent.x"
235+
[minMaxStep]="minMaxStep"
235236
[axisSize]="domDim.width"
236237
(onTimeSelectionChanged)="onTimeSelectionChanged.emit($event)"
237238
(onTimeSelectionToggled)="onFobRemoved()"

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
TimeSelectionAffordance,
2525
} from '../../../widgets/card_fob/card_fob_types';
2626
import {Scale} from '../../../widgets/line_chart_v2/lib/public_types';
27+
import {MinMaxStep} from './scalar_card_types';
2728

2829
@Component({
2930
selector: 'scalar-card-fob-controller',
@@ -47,7 +48,8 @@ import {Scale} from '../../../widgets/line_chart_v2/lib/public_types';
4748
export class ScalarCardFobController {
4849
@Input() timeSelection!: TimeSelection;
4950
@Input() scale!: Scale;
50-
@Input() minMax!: [number, number];
51+
@Input() minMaxHorizontalViewExtend!: [number, number];
52+
@Input() minMaxStep!: MinMaxStep;
5153
@Input() axisSize!: number;
5254

5355
@Output() onTimeSelectionChanged = new EventEmitter<{
@@ -65,7 +67,7 @@ export class ScalarCardFobController {
6567

6668
getAxisPositionFromStartStep() {
6769
return this.scale.forward(
68-
this.minMax,
70+
this.minMaxHorizontalViewExtend,
6971
[0, this.axisSize],
7072
this.timeSelection.start.step
7173
);
@@ -76,20 +78,18 @@ export class ScalarCardFobController {
7678
return null;
7779
}
7880
return this.scale.forward(
79-
this.minMax,
81+
this.minMaxHorizontalViewExtend,
8082
[0, this.axisSize],
8183
this.timeSelection.end.step
8284
);
8385
}
8486

8587
getHighestStep(): number {
86-
const minMax = this.minMax;
87-
return Math.floor(minMax[0] < minMax[1] ? minMax[1] : minMax[0]);
88+
return this.minMaxStep.maxStep;
8889
}
8990

9091
getLowestStep(): number {
91-
const minMax = this.minMax;
92-
return Math.ceil(minMax[0] < minMax[1] ? minMax[0] : minMax[1]);
92+
return this.minMaxStep.minStep;
9393
}
9494

9595
getStepHigherThanAxisPosition(position: number): number {
@@ -113,7 +113,11 @@ export class ScalarCardFobController {
113113
*/
114114
getStepAtMousePostion(position: number): number {
115115
const stepAtMouse = Math.round(
116-
this.scale.reverse(this.minMax, [0, this.axisSize], position)
116+
this.scale.reverse(
117+
this.minMaxHorizontalViewExtend,
118+
[0, this.axisSize],
119+
position
120+
)
117121
);
118122
if (stepAtMouse > this.getHighestStep()) {
119123
return this.getHighestStep();

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

Lines changed: 19 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {CardFobControllerComponent} from '../../../widgets/card_fob/card_fob_con
1616
import {TimeSelection} from '../../../widgets/card_fob/card_fob_types';
1717
import {LinearScale} from '../../../widgets/line_chart_v2/lib/scale';
1818
import {ScalarCardFobController} from './scalar_card_fob_controller';
19+
import {MinMaxStep} from './scalar_card_types';
1920

2021
const SCALE_RATIO = 10;
2122

@@ -33,6 +34,7 @@ describe('ScalarFobController', () => {
3334
function createComponent(input: {
3435
timeSelection?: TimeSelection;
3536
minMax?: [number, number];
37+
minMaxStep?: MinMaxStep;
3638
axisSize?: number;
3739
}): ComponentFixture<ScalarCardFobController> {
3840
const fixture = TestBed.createComponent(ScalarCardFobController);
@@ -41,7 +43,13 @@ describe('ScalarFobController', () => {
4143
end: null,
4244
};
4345

44-
fixture.componentInstance.minMax = input.minMax ?? [0, 1];
46+
fixture.componentInstance.minMaxHorizontalViewExtend = input.minMax ?? [
47+
-1, 2,
48+
];
49+
fixture.componentInstance.minMaxStep = input.minMaxStep ?? {
50+
minStep: 0,
51+
maxStep: 1,
52+
};
4553
fixture.componentInstance.axisSize = input.axisSize ?? 1;
4654

4755
const fakeScale = new LinearScale();
@@ -89,56 +97,14 @@ describe('ScalarFobController', () => {
8997
});
9098
});
9199

92-
describe('getHighestStep', () => {
93-
it('gets the highest step when minMax is in order', () => {
94-
const fixture = createComponent({minMax: [0, 2]});
95-
96-
const highestStep = fixture.componentInstance.getHighestStep();
97-
98-
expect(highestStep).toBe(2);
99-
});
100-
101-
it('gets the highest step when minMax is not in order', () => {
102-
const fixture = createComponent({minMax: [2, 0]});
103-
104-
const highestStep = fixture.componentInstance.getHighestStep();
100+
it('gets the highest/lowest step', () => {
101+
const fixture = createComponent({minMaxStep: {minStep: 0, maxStep: 2}});
105102

106-
expect(highestStep).toBe(2);
107-
});
108-
109-
it('returns the next lowest step when max is a floating point.', () => {
110-
const fixture = createComponent({minMax: [4.5, 1]});
111-
112-
const lowestStep = fixture.componentInstance.getHighestStep();
113-
114-
expect(lowestStep).toBe(4);
115-
});
116-
});
103+
const highestStep = fixture.componentInstance.getHighestStep();
104+
const lowestStep = fixture.componentInstance.getLowestStep();
117105

118-
describe('getLowestStep', () => {
119-
it('gets the lowest step when minMax is in order', () => {
120-
const fixture = createComponent({minMax: [0, 2]});
121-
122-
const lowestStep = fixture.componentInstance.getLowestStep();
123-
124-
expect(lowestStep).toBe(0);
125-
});
126-
127-
it('gets the lowest step when minMax is not in order', () => {
128-
const fixture = createComponent({minMax: [2, 0]});
129-
130-
const lowestStep = fixture.componentInstance.getLowestStep();
131-
132-
expect(lowestStep).toBe(0);
133-
});
134-
135-
it('returns the next highest step when min is a floating point.', () => {
136-
const fixture = createComponent({minMax: [4, 1.5]});
137-
138-
const lowestStep = fixture.componentInstance.getLowestStep();
139-
140-
expect(lowestStep).toBe(2);
141-
});
106+
expect(lowestStep).toBe(0);
107+
expect(highestStep).toBe(2);
142108
});
143109

144110
describe('getStepHigherThanAxisPosition', () => {
@@ -151,15 +117,15 @@ describe('ScalarFobController', () => {
151117
});
152118

153119
it('gets highest step if given position is higher than the position at highest step', () => {
154-
const fixture = createComponent({minMax: [0, 2]});
120+
const fixture = createComponent({minMaxStep: {minStep: 0, maxStep: 2}});
155121

156122
const step = fixture.componentInstance.getStepHigherThanAxisPosition(30);
157123

158124
expect(step).toBe(2);
159125
});
160126

161127
it('gets lowest step if given position is lower than the position at lowest step', () => {
162-
const fixture = createComponent({minMax: [1, 3]});
128+
const fixture = createComponent({minMaxStep: {minStep: 1, maxStep: 3}});
163129

164130
const step = fixture.componentInstance.getStepHigherThanAxisPosition(0);
165131

@@ -177,15 +143,15 @@ describe('ScalarFobController', () => {
177143
});
178144

179145
it('gets highest step if given position is higher than the position at highest step', () => {
180-
const fixture = createComponent({minMax: [0, 2]});
146+
const fixture = createComponent({minMaxStep: {minStep: 0, maxStep: 2}});
181147

182148
const step = fixture.componentInstance.getStepLowerThanAxisPosition(30);
183149

184150
expect(step).toBe(2);
185151
});
186152

187153
it('gets lowest step if given position is lower than the position at lowest step', () => {
188-
const fixture = createComponent({minMax: [1, 3]});
154+
const fixture = createComponent({minMaxStep: {minStep: 1, maxStep: 3}});
189155

190156
const step = fixture.componentInstance.getStepLowerThanAxisPosition(0);
191157

0 commit comments

Comments
 (0)