Skip to content

Commit addbdd0

Browse files
authored
Merge pull request #5103 from VisActor/5102-bug-aggregation-float
5102 bug aggregation float
2 parents 7f44a5d + b23408c commit addbdd0

4 files changed

Lines changed: 118 additions & 69 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "fix: aggregation precisionAdd precisionSub\n\n",
5+
"type": "none",
6+
"packageName": "@visactor/vtable"
7+
}
8+
],
9+
"packageName": "@visactor/vtable",
10+
"email": "892739385@qq.com"
11+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { SumAggregator } from '../../src/ts-types/dataset/aggregation';
2+
3+
describe('SumAggregator floating point precision', () => {
4+
it('should sum 0.1 and 0.2 exactly to 0.3', () => {
5+
const aggregator = new SumAggregator({ key: 'test', field: 'value', isRecord: false });
6+
aggregator.push({ value: 0.1 });
7+
aggregator.push({ value: 0.2 });
8+
expect(aggregator.value()).toBe(0.3);
9+
});
10+
11+
it('should subtract correctly', () => {
12+
const aggregator = new SumAggregator({ key: 'test', field: 'value', isRecord: false });
13+
aggregator.push({ value: 0.3 });
14+
aggregator.deleteRecord({ value: 0.1 });
15+
expect(aggregator.value()).toBe(0.2);
16+
});
17+
});

packages/vtable/__tests__/pivotChart.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9487,7 +9487,7 @@ describe('pivotChart init test', () => {
94879487
},
94889488
二级: {
94899489
max: 81585.58783792704,
9490-
min: -15135.001037597656,
9490+
min: -15135.001037597654,
94919491
positiveMax: 81585.58783792704,
94929492
negativeMin: -19659.724044799805
94939493
},
@@ -9598,7 +9598,7 @@ describe('pivotChart init test', () => {
95989598
},
95999599
'230713150305011': {
96009600
一级: {
9601-
max: 40780.32046222687,
9601+
max: 40780.32046222686,
96029602
min: -22801.40795326233,
96039603
positiveMax: 44028.34812831879,
96049604
negativeMin: -22801.40795326233

packages/vtable/src/ts-types/dataset/aggregation.ts

Lines changed: 88 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isValid } from '@visactor/vutils';
1+
import { isValid, precisionAdd, precisionSub } from '@visactor/vutils';
22
import type { SortOrder } from '..';
33
import { AggregationType, SortType } from '..';
44
import type { BaseTableAPI } from '../base-table';
@@ -409,22 +409,22 @@ export class SumAggregator extends Aggregator {
409409
if (record.isAggregator && this.children) {
410410
this.children.push(record);
411411
const value = record.value();
412-
this.sum += value ?? 0;
412+
this.sum = precisionAdd(this.sum, value ?? 0);
413413
if (this.needSplitPositiveAndNegativeForSum) {
414414
if (value > 0) {
415-
this.positiveSum += value;
415+
this.positiveSum = precisionAdd(this.positiveSum, value);
416416
} else if (value < 0) {
417-
this.nagetiveSum += value;
417+
this.nagetiveSum = precisionAdd(this.nagetiveSum, value);
418418
}
419419
}
420420
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
421421
const value = parseFloat(record[this.field]);
422-
this.sum += value;
422+
this.sum = precisionAdd(this.sum, value);
423423
if (this.needSplitPositiveAndNegativeForSum) {
424424
if (value > 0) {
425-
this.positiveSum += value;
425+
this.positiveSum = precisionAdd(this.positiveSum, value);
426426
} else if (value < 0) {
427-
this.nagetiveSum += value;
427+
this.nagetiveSum = precisionAdd(this.nagetiveSum, value);
428428
}
429429
}
430430
}
@@ -434,27 +434,33 @@ export class SumAggregator extends Aggregator {
434434
deleteRecord(record: any) {
435435
if (record) {
436436
if (this.isRecord && this.records) {
437-
this.records = this.records.filter(item => item !== record);
437+
const index = this.records.indexOf(record);
438+
if (index !== -1) {
439+
this.records.splice(index, 1);
440+
}
438441
}
439442
if (record.isAggregator && this.children) {
440-
this.children = this.children.filter(item => item !== record);
443+
const index = this.children.indexOf(record);
444+
if (index !== -1) {
445+
this.children.splice(index, 1);
446+
}
441447
const value = record.value();
442-
this.sum -= value ?? 0;
448+
this.sum = precisionSub(this.sum, value ?? 0);
443449
if (this.needSplitPositiveAndNegativeForSum) {
444450
if (value > 0) {
445-
this.positiveSum -= value;
451+
this.positiveSum = precisionSub(this.positiveSum, value);
446452
} else if (value < 0) {
447-
this.nagetiveSum -= value;
453+
this.nagetiveSum = precisionSub(this.nagetiveSum, value);
448454
}
449455
}
450456
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
451457
const value = parseFloat(record[this.field]);
452-
this.sum -= value;
458+
this.sum = precisionSub(this.sum, value);
453459
if (this.needSplitPositiveAndNegativeForSum) {
454460
if (value > 0) {
455-
this.positiveSum -= value;
461+
this.positiveSum = precisionSub(this.positiveSum, value);
456462
} else if (value < 0) {
457-
this.nagetiveSum -= value;
463+
this.nagetiveSum = precisionSub(this.nagetiveSum, value);
458464
}
459465
}
460466
}
@@ -464,53 +470,56 @@ export class SumAggregator extends Aggregator {
464470
updateRecord(oldRecord: any, newRecord: any): void {
465471
if (oldRecord && newRecord) {
466472
if (this.isRecord && this.records) {
467-
this.records = this.records.map(item => {
468-
if (item === oldRecord) {
469-
return newRecord;
470-
}
471-
return item;
472-
});
473+
const index = this.records.indexOf(oldRecord);
474+
if (index !== -1) {
475+
this.records[index] = newRecord;
476+
}
473477
}
474478
if (oldRecord.isAggregator && this.children) {
475479
const oldValue = oldRecord.value();
476-
this.children = this.children.filter(item => item !== oldRecord);
480+
const index = this.children.indexOf(oldRecord);
481+
if (index !== -1) {
482+
this.children[index] = newRecord;
483+
}
477484
const newValue = newRecord.value();
478-
this.children.push(newRecord);
479-
this.sum += newValue - oldValue;
485+
this.sum = precisionAdd(this.sum, precisionSub(newValue, oldValue));
480486
if (this.needSplitPositiveAndNegativeForSum) {
481487
if (oldValue > 0) {
482-
this.positiveSum -= oldValue;
488+
this.positiveSum = precisionSub(this.positiveSum, oldValue);
483489
} else if (oldValue < 0) {
484-
this.nagetiveSum -= oldValue;
490+
this.nagetiveSum = precisionSub(this.nagetiveSum, oldValue);
485491
}
486492
if (newValue > 0) {
487-
this.positiveSum += newValue;
493+
this.positiveSum = precisionAdd(this.positiveSum, newValue);
488494
} else if (newValue < 0) {
489-
this.nagetiveSum += newValue;
495+
this.nagetiveSum = precisionAdd(this.nagetiveSum, newValue);
490496
}
491497
}
492498
} else if (this.field && !isNaN(parseFloat(oldRecord[this.field]))) {
493499
const oldValue = parseFloat(oldRecord[this.field]);
494500
const newValue = parseFloat(newRecord[this.field]);
495-
this.sum += newValue - oldValue;
501+
this.sum = precisionAdd(this.sum, precisionSub(newValue, oldValue));
496502
if (this.needSplitPositiveAndNegativeForSum) {
497503
if (oldValue > 0) {
498-
this.positiveSum -= oldValue;
504+
this.positiveSum = precisionSub(this.positiveSum, oldValue);
499505
} else if (oldValue < 0) {
500-
this.nagetiveSum -= oldValue;
506+
this.nagetiveSum = precisionSub(this.nagetiveSum, oldValue);
501507
}
502508
if (newValue > 0) {
503-
this.positiveSum += newValue;
509+
this.positiveSum = precisionAdd(this.positiveSum, newValue);
504510
} else if (newValue < 0) {
505-
this.nagetiveSum += newValue;
511+
this.nagetiveSum = precisionAdd(this.nagetiveSum, newValue);
506512
}
507513
}
508514
}
509515
this.clearCacheValue();
510516
}
511517
}
512518
value() {
513-
return this.changedValue ?? (this.records?.length >= 1 ? this.sum : undefined);
519+
return (
520+
this.changedValue ??
521+
(this.records && this.records.length >= 1 ? this.sum : this.isRecord === false ? this.sum : undefined)
522+
);
514523
}
515524
positiveValue() {
516525
return this.positiveSum;
@@ -531,12 +540,12 @@ export class SumAggregator extends Aggregator {
531540
const child = this.children[i];
532541
if (child.isAggregator) {
533542
const value = child.value();
534-
this.sum += value ?? 0;
543+
this.sum = precisionAdd(this.sum, value ?? 0);
535544
if (this.needSplitPositiveAndNegativeForSum) {
536545
if (value > 0) {
537-
this.positiveSum += value;
546+
this.positiveSum = precisionAdd(this.positiveSum, value);
538547
} else if (value < 0) {
539-
this.nagetiveSum += value;
548+
this.nagetiveSum = precisionAdd(this.nagetiveSum, value);
540549
}
541550
}
542551
}
@@ -546,22 +555,22 @@ export class SumAggregator extends Aggregator {
546555
const record = this.records[i];
547556
if (record.isAggregator) {
548557
const value = record.value();
549-
this.sum += value ?? 0;
558+
this.sum = precisionAdd(this.sum, value ?? 0);
550559
if (this.needSplitPositiveAndNegativeForSum) {
551560
if (value > 0) {
552-
this.positiveSum += value;
561+
this.positiveSum = precisionAdd(this.positiveSum, value);
553562
} else if (value < 0) {
554-
this.nagetiveSum += value;
563+
this.nagetiveSum = precisionAdd(this.nagetiveSum, value);
555564
}
556565
}
557566
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
558567
const value = parseFloat(record[this.field]);
559-
this.sum += value;
568+
this.sum = precisionAdd(this.sum, value);
560569
if (this.needSplitPositiveAndNegativeForSum) {
561570
if (value > 0) {
562-
this.positiveSum += value;
571+
this.positiveSum = precisionAdd(this.positiveSum, value);
563572
} else if (value < 0) {
564-
this.nagetiveSum += value;
573+
this.nagetiveSum = precisionAdd(this.nagetiveSum, value);
565574
}
566575
}
567576
}
@@ -687,10 +696,10 @@ export class AvgAggregator extends Aggregator {
687696
if (this.children) {
688697
this.children.push(record);
689698
}
690-
this.sum += record.sum;
699+
this.sum = precisionAdd(this.sum, record.sum);
691700
this.count += record.count;
692701
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
693-
this.sum += parseFloat(record[this.field]);
702+
this.sum = precisionAdd(this.sum, parseFloat(record[this.field]));
694703
this.count++;
695704
}
696705
}
@@ -699,16 +708,22 @@ export class AvgAggregator extends Aggregator {
699708
deleteRecord(record: any) {
700709
if (record) {
701710
if (this.isRecord && this.records) {
702-
this.records = this.records.filter(item => item !== record);
711+
const index = this.records.indexOf(record);
712+
if (index !== -1) {
713+
this.records.splice(index, 1);
714+
}
703715
}
704716
if (record.isAggregator && record.type === AggregationType.AVG) {
705717
if (this.children) {
706-
this.children = this.children.filter(item => item !== record);
718+
const index = this.children.indexOf(record);
719+
if (index !== -1) {
720+
this.children.splice(index, 1);
721+
}
707722
}
708-
this.sum -= record.sum;
723+
this.sum = precisionSub(this.sum, record.sum);
709724
this.count -= record.count;
710725
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
711-
this.sum -= parseFloat(record[this.field]);
726+
this.sum = precisionSub(this.sum, parseFloat(record[this.field]));
712727
this.count--;
713728
}
714729
}
@@ -717,33 +732,39 @@ export class AvgAggregator extends Aggregator {
717732
updateRecord(oldRecord: any, newRecord: any): void {
718733
if (oldRecord && newRecord) {
719734
if (this.isRecord && this.records) {
720-
this.records = this.records.map(item => {
721-
if (item === oldRecord) {
722-
return newRecord;
723-
}
724-
return item;
725-
});
735+
const index = this.records.indexOf(oldRecord);
736+
if (index !== -1) {
737+
this.records[index] = newRecord;
738+
}
726739
}
727740
if (oldRecord.isAggregator && oldRecord.type === AggregationType.AVG) {
728741
if (this.children && newRecord.isAggregator) {
729-
this.children = this.children.map(item => {
730-
if (item === oldRecord) {
731-
return newRecord;
732-
}
733-
return item;
734-
});
742+
const index = this.children.indexOf(oldRecord);
743+
if (index !== -1) {
744+
this.children[index] = newRecord;
745+
}
735746
}
736-
this.sum += newRecord.sum - oldRecord.sum;
747+
this.sum = precisionAdd(this.sum, precisionSub(newRecord.sum, oldRecord.sum));
737748
this.count += newRecord.count - oldRecord.count;
738749
} else if (this.field && !isNaN(parseFloat(oldRecord[this.field]))) {
739-
this.sum += parseFloat(newRecord[this.field]) - parseFloat(oldRecord[this.field]);
750+
this.sum = precisionAdd(
751+
this.sum,
752+
precisionSub(parseFloat(newRecord[this.field]), parseFloat(oldRecord[this.field]))
753+
);
740754
// this.count++;
741755
}
742756
this.clearCacheValue();
743757
}
744758
}
745759
value() {
746-
return this.changedValue ?? (this.records?.length >= 1 ? this.sum / this.count : undefined);
760+
return (
761+
this.changedValue ??
762+
(this.records && this.records.length >= 1
763+
? this.sum / this.count
764+
: this.isRecord === false && this.count > 0
765+
? this.sum / this.count
766+
: undefined)
767+
);
747768
}
748769
reset() {
749770
this.changedValue = undefined;
@@ -761,18 +782,18 @@ export class AvgAggregator extends Aggregator {
761782
const child = this.children[i];
762783
if (child.isAggregator && child.type === AggregationType.AVG) {
763784
const childValue = child.value();
764-
this.sum += childValue * (child as AvgAggregator).count;
785+
this.sum = precisionAdd(this.sum, childValue * (child as AvgAggregator).count);
765786
this.count += (child as AvgAggregator).count;
766787
}
767788
}
768789
} else if (this.records) {
769790
for (let i = 0; i < this.records.length; i++) {
770791
const record = this.records[i];
771792
if (record.isAggregator && record.type === AggregationType.AVG) {
772-
this.sum += record.sum;
793+
this.sum = precisionAdd(this.sum, record.sum);
773794
this.count += record.count;
774795
} else if (this.field && !isNaN(parseFloat(record[this.field]))) {
775-
this.sum += parseFloat(record[this.field]);
796+
this.sum = precisionAdd(this.sum, parseFloat(record[this.field]));
776797
this.count++;
777798
}
778799
}

0 commit comments

Comments
 (0)