Skip to content

Commit b060821

Browse files
authored
fix: Display properly rounded subset counts (#6748)
1 parent b83790b commit b060821

6 files changed

Lines changed: 159 additions & 49 deletions

File tree

application/ui/src/features/models/model-listing/components/three-section-range/three-section-range.component.tsx

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
import { Flex, Text } from '@geti/ui';
55

6+
import { distributeByLargestRemainder } from '../../../utils';
7+
68
import classes from './three-section-range.module.scss';
79

810
type ThreeSectionRangeProps = {
@@ -12,38 +14,13 @@ type ThreeSectionRangeProps = {
1214
testingValue: number;
1315
};
1416

15-
// Distribute rounded percentages so that they always sum to 100%
16-
const computeRoundedPercentages = (values: number[]): number[] => {
17-
const total = values.reduce((sum, value) => sum + value, 0);
18-
19-
if (total <= 0) {
20-
return values.map(() => 0);
21-
}
22-
23-
const exactPercentages = values.map((value) => (value * 100) / total);
24-
const flooredPercentages = exactPercentages.map((percentage) => Math.floor(percentage));
25-
let remainder = 100 - flooredPercentages.reduce((sum, value) => sum + value, 0);
26-
27-
const indicesByRemainder = exactPercentages
28-
.map((percentage, index) => ({ index, fractional: percentage - Math.floor(percentage) }))
29-
.sort((a, b) => b.fractional - a.fractional);
30-
31-
const result = [...flooredPercentages];
32-
for (const { index } of indicesByRemainder) {
33-
if (remainder <= 0) break;
34-
result[index] += 1;
35-
remainder -= 1;
36-
}
37-
38-
return result;
39-
};
17+
const MAX_VALUE = 100;
4018

4119
export const ThreeSectionRange = ({ id, trainingValue, validationValue, testingValue }: ThreeSectionRangeProps) => {
42-
const [trainingPercentage, validationPercentage, testingPercentage] = computeRoundedPercentages([
43-
trainingValue,
44-
validationValue,
45-
testingValue,
46-
]);
20+
const [trainingPercentage, validationPercentage, testingPercentage] = distributeByLargestRemainder(
21+
[trainingValue, validationValue, testingValue],
22+
MAX_VALUE
23+
);
4724

4825
const labelledPercentages = [
4926
{ label: 'Training', percentage: trainingPercentage, color: 'var(--training-subset)' },

application/ui/src/features/models/train-model/advanced-settings/data-management/training-subsets/resulting-dataset-distribution.component.tsx

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33

44
import { Flex, Grid, repeat, Text, View } from '@geti/ui';
55

6+
import { distributeByLargestRemainder } from '../../../../utils';
67
import { LABEL_COLOR_MAPPING, SubsetTile } from './subset-distribution-stats.component';
78

89
import classes from './training-subsets.module.scss';
910

1011
type SubsetDistributionRowProps = {
1112
existingSize: number;
1213
newSize: number;
13-
totalSize: number;
1414
label: string;
15+
percentage: number;
1516
};
1617

1718
const SubsetLabel = ({ label, color }: { color: string; label: string }) => {
@@ -23,9 +24,8 @@ const SubsetLabel = ({ label, color }: { color: string; label: string }) => {
2324
);
2425
};
2526

26-
const SubsetDistributionRow = ({ existingSize, newSize, totalSize, label }: SubsetDistributionRowProps) => {
27+
const SubsetDistributionRow = ({ existingSize, newSize, label, percentage }: SubsetDistributionRowProps) => {
2728
const resultingSize = existingSize + newSize;
28-
const percentage = totalSize > 0 ? Math.round((resultingSize * 100) / totalSize) : 0;
2929

3030
return (
3131
<>
@@ -42,23 +42,28 @@ const SubsetDistributionRow = ({ existingSize, newSize, totalSize, label }: Subs
4242
type ResultingDatasetDistributionSubsetProps = {
4343
color: string;
4444
label: string;
45-
totalSize: number;
4645
newSize: number;
4746
existingSize: number;
47+
percentage: number;
4848
};
4949

5050
const ResultingDatasetDistributionSubset = ({
5151
color,
5252
label,
5353
existingSize,
5454
newSize,
55-
totalSize,
55+
percentage,
5656
}: ResultingDatasetDistributionSubsetProps) => {
5757
return (
5858
<>
5959
<SubsetLabel label={label} color={color} />
6060

61-
<SubsetDistributionRow label={label} totalSize={totalSize} newSize={newSize} existingSize={existingSize} />
61+
<SubsetDistributionRow
62+
label={label}
63+
newSize={newSize}
64+
existingSize={existingSize}
65+
percentage={percentage}
66+
/>
6267
</>
6368
);
6469
};
@@ -70,18 +75,27 @@ type ResultingDatasetDistributionProps = {
7075
newTrainingSubsetSize: number;
7176
newValidationSubsetSize: number;
7277
newTestingSubsetSize: number;
73-
totalDatasetItemsSize: number;
7478
};
7579

80+
const MAX_VALUE = 100;
81+
7682
export const ResultingDatasetDistribution = ({
7783
trainingSubsetSize,
7884
validationSubsetSize,
7985
testingSubsetSize,
8086
newTrainingSubsetSize,
8187
newValidationSubsetSize,
8288
newTestingSubsetSize,
83-
totalDatasetItemsSize,
8489
}: ResultingDatasetDistributionProps) => {
90+
const [trainingPercentage, validationPercentage, testingPercentage] = distributeByLargestRemainder(
91+
[
92+
trainingSubsetSize + newTrainingSubsetSize,
93+
validationSubsetSize + newValidationSubsetSize,
94+
testingSubsetSize + newTestingSubsetSize,
95+
],
96+
MAX_VALUE
97+
);
98+
8599
return (
86100
<Flex direction={'column'} gap={'size-50'}>
87101
<Text>Resulting dataset distribution:</Text>
@@ -96,25 +110,25 @@ export const ResultingDatasetDistribution = ({
96110
<ResultingDatasetDistributionSubset
97111
label={'Training'}
98112
color={LABEL_COLOR_MAPPING.training}
99-
totalSize={totalDatasetItemsSize}
100113
newSize={newTrainingSubsetSize}
101114
existingSize={trainingSubsetSize}
115+
percentage={trainingPercentage}
102116
/>
103117

104118
<ResultingDatasetDistributionSubset
105119
label={'Validation'}
106120
color={LABEL_COLOR_MAPPING.validation}
107-
totalSize={totalDatasetItemsSize}
108121
newSize={newValidationSubsetSize}
109122
existingSize={validationSubsetSize}
123+
percentage={validationPercentage}
110124
/>
111125

112126
<ResultingDatasetDistributionSubset
113127
label={'Test'}
114128
color={LABEL_COLOR_MAPPING.test}
115-
totalSize={totalDatasetItemsSize}
116129
newSize={newTestingSubsetSize}
117130
existingSize={testingSubsetSize}
131+
percentage={testingPercentage}
118132
/>
119133
</Grid>
120134
</View>

application/ui/src/features/models/train-model/advanced-settings/data-management/training-subsets/training-subsets.component.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { isEqual } from 'lodash-es';
99

1010
import type { ConfigurableParameter, TrainingConfiguration } from '../../../../../../constants/shared-types';
1111
import { isParameterGroup } from '../../../../model-listing/model-training-parameters/utils';
12+
import { distributeByLargestRemainder } from '../../../../utils';
1213
import { Accordion } from '../../components/accordion/accordion.component';
1314
import { ResultingDatasetDistribution } from './resulting-dataset-distribution.component';
1415
import { SubsetsDistribution } from './subset-distribution.component';
@@ -163,9 +164,10 @@ export const TrainingSubsets = ({
163164
});
164165
};
165166

166-
const newValidationSubsetSize = Math.floor((validationSubsetRatio / 100) * unassignedSubsetSize);
167-
const newTestingSubsetSize = Math.floor((testSubsetRatio / 100) * unassignedSubsetSize);
168-
const newTrainingSubsetSize = unassignedSubsetSize - newValidationSubsetSize - newTestingSubsetSize;
167+
const [newTrainingSubsetSize, newValidationSubsetSize, newTestingSubsetSize] = distributeByLargestRemainder(
168+
[trainingSubsetRatio, validationSubsetRatio, testSubsetRatio],
169+
unassignedSubsetSize
170+
);
169171

170172
const areSubsetsSizesValid = () => {
171173
const resultingTrainingSubsetSize = trainingSubsetSize + newTrainingSubsetSize;
@@ -217,7 +219,6 @@ export const TrainingSubsets = ({
217219
newTrainingSubsetSize={newTrainingSubsetSize}
218220
newValidationSubsetSize={newValidationSubsetSize}
219221
newTestingSubsetSize={newTestingSubsetSize}
220-
totalDatasetItemsSize={totalDatasetItemsSize}
221222
/>
222223
</View>
223224

application/ui/src/features/models/train-model/advanced-settings/data-management/training-subsets/training-subsets.test.tsx

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { render } from 'test-utils/render';
1414
import { http } from '../../../../../../api/utils';
1515
import { TrainingConfiguration } from '../../../../../../constants/shared-types';
1616
import { server } from '../../../../../../msw-node-setup';
17+
import { distributeByLargestRemainder } from '../../../../utils';
1718
import { TrainingSubsets } from './training-subsets.component';
1819
import { getSubsetSplitParameters, SubsetSplitParameters } from './utils';
1920

@@ -108,9 +109,10 @@ const expectTrainingSubsetsDistribution = async ({
108109
validationSubset: number;
109110
testSubset: number;
110111
}) => {
111-
const newTestingSize = Math.floor(unassignedSize * (testSubset / 100));
112-
const newValidationSize = Math.floor(unassignedSize * (validationSubset / 100));
113-
const newTrainingSize = unassignedSize - newValidationSize - newTestingSize;
112+
const [newTrainingSize, newValidationSize, newTestingSize] = distributeByLargestRemainder(
113+
[trainingSubset, validationSubset, testSubset],
114+
unassignedSize
115+
);
114116

115117
expectTrainingSubsetsDistributionProportion({
116118
validationSubset,
@@ -393,4 +395,50 @@ describe('TrainingSubsets', () => {
393395
expect(alert).toBeInTheDocument();
394396
expect(within(alert).getByRole('heading')).toHaveTextContent('Invalid training subsets configuration');
395397
});
398+
399+
it('45/28/27 split on 5 unassigned items distributes as Training=2, Validation=2, Test=1', async () => {
400+
const trainingSize = 0;
401+
const validationSize = 0;
402+
const testSize = 0;
403+
const unassignedSize = 5;
404+
405+
mockSubsetsNetworkRequest({ trainingSize, validationSize, unassignedSize, testSize });
406+
407+
const regressionSubsetParameters: SubsetSplitParameters = [
408+
getMockedConfigurationParameter({
409+
key: 'training',
410+
value_type: 'int',
411+
name: 'Training percentage',
412+
value: 45,
413+
description: 'Percentage of data to use for training',
414+
default_value: 45,
415+
max_value: 100,
416+
min_value: 1,
417+
}),
418+
getMockedConfigurationParameter({
419+
key: 'validation',
420+
value_type: 'int',
421+
name: 'Validation percentage',
422+
value: 28,
423+
description: 'Percentage of data to use for validation',
424+
default_value: 28,
425+
max_value: 100,
426+
min_value: 1,
427+
}),
428+
getMockedConfigurationParameter({
429+
key: 'test',
430+
value_type: 'int',
431+
name: 'Test percentage',
432+
value: 27,
433+
description: 'Percentage of data to use for testing',
434+
default_value: 27,
435+
max_value: 100,
436+
min_value: 1,
437+
}),
438+
];
439+
440+
render(<App subsetParameters={regressionSubsetParameters} />);
441+
442+
await expectSubsetSizes({ trainingSize: 2, validationSize: 2, testSize: 1 });
443+
});
396444
});

application/ui/src/features/models/utils.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,52 @@
44
import { getMockedModel } from 'mocks/mock-model';
55
import { getMockedVariant } from 'mocks/mock-model-variant';
66

7-
import { getAllModelsWithOpenVINOVariants, getModelIdentifierPayload, SelectableModel } from './utils';
7+
import {
8+
distributeByLargestRemainder,
9+
getAllModelsWithOpenVINOVariants,
10+
getModelIdentifierPayload,
11+
SelectableModel,
12+
} from './utils';
13+
14+
describe('distributeByLargestRemainder', () => {
15+
it('returns all zeros when total is zero', () => {
16+
expect(distributeByLargestRemainder([70, 20, 10], 0)).toEqual([0, 0, 0]);
17+
});
18+
19+
it('returns all zeros when sum of values is zero', () => {
20+
expect(distributeByLargestRemainder([0, 0, 0], 100)).toEqual([0, 0, 0]);
21+
});
22+
23+
it('returns empty array for empty input with non-zero total', () => {
24+
expect(distributeByLargestRemainder([], 100)).toEqual([]);
25+
});
26+
27+
it('result always sums to total', () => {
28+
const result = distributeByLargestRemainder([750, 125, 125], 100);
29+
expect(result.reduce((a, b) => a + b, 0)).toBe(100);
30+
expect(result).toEqual([75, 13, 12]);
31+
});
32+
33+
it('tie-breaking: largest fractional part gets the extra unit', () => {
34+
// [1, 1, 1] → each gets 33.33%, floors to 33, remainder = 1
35+
// all fractionals equal → first in sorted order wins
36+
const result = distributeByLargestRemainder([1, 1, 1], 100);
37+
expect(result.reduce((a, b) => a + b, 0)).toBe(100);
38+
expect(result).toEqual([34, 33, 33]);
39+
});
40+
41+
it('count case: 45/28/27 split on 5 items → [2, 2, 1]', () => {
42+
expect(distributeByLargestRemainder([45, 28, 27], 5)).toEqual([2, 2, 1]);
43+
});
44+
45+
it('standard percentage case: 70/20/10 on 100 → [70, 20, 10]', () => {
46+
expect(distributeByLargestRemainder([70, 20, 10], 100)).toEqual([70, 20, 10]);
47+
});
48+
49+
it('handles a single value', () => {
50+
expect(distributeByLargestRemainder([5], 100)).toEqual([100]);
51+
});
52+
});
853

954
describe('getAllModelsWithOpenVINOVariants', () => {
1055
it('returns empty array for empty models array', () => {

application/ui/src/features/models/utils.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,31 @@ export const getModelIdentifierPayload = (model: SelectableModel): { model_id: s
1010
model_variant_id: model.modelVariantId,
1111
});
1212

13+
export const distributeByLargestRemainder = (values: number[], total: number): number[] => {
14+
const sum = values.reduce((acc, value) => acc + value, 0);
15+
16+
if (sum <= 0 || total <= 0) {
17+
return values.map(() => 0);
18+
}
19+
20+
const exactShares = values.map((value) => (value / sum) * total);
21+
const flooredShares = exactShares.map((share) => Math.floor(share));
22+
let remainder = total - flooredShares.reduce((acc, value) => acc + value, 0);
23+
24+
const indicesByRemainder = exactShares
25+
.map((share, index) => ({ index, fractional: share - Math.floor(share) }))
26+
.sort((a, b) => b.fractional - a.fractional);
27+
28+
const result = [...flooredShares];
29+
for (const { index } of indicesByRemainder) {
30+
if (remainder <= 0) break;
31+
result[index] += 1;
32+
remainder -= 1;
33+
}
34+
35+
return result;
36+
};
37+
1338
export const getAllModelsWithOpenVINOVariants = (models: Model[]): SelectableModel[] => {
1439
return models.flatMap((model) =>
1540
model.variants

0 commit comments

Comments
 (0)