Skip to content

Commit

Permalink
fix(formula): facade api test, floating point calculation tolerance
Browse files Browse the repository at this point in the history
  • Loading branch information
Dushusir committed Aug 22, 2024
1 parent dd8b221 commit a927229
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,13 @@ describe('Test math kit', () => {
});

it('Function getFractionalPart', () => {
expect(stripErrorMargin(getFractionalPart(123.456))).toBe(0.456000000000003);
expect(stripErrorMargin(getFractionalPart(-123.456))).toBe(-0.456000000000003);
expect(stripErrorMargin(getFractionalPart(123.456))).toBe(0.456);
expect(stripErrorMargin(getFractionalPart(-123.456))).toBe(-0.456);
expect(getFractionalPart(123)).toBe(0);
});

it('Function stripErrorMargin', () => {
expect(stripErrorMargin(0.1 + 0.2)).toBe(0.3);
expect(stripErrorMargin(30.2 - 30)).toBe(0.2);
});
});
16 changes: 11 additions & 5 deletions packages/engine-formula/src/engine/utils/math-kit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,27 @@ export function strip(num: number, precision = 15) {
* @param right
* @returns
*/
export function withinErrorMargin(left: number, right: number) {
return Math.abs(left - right) < Number.EPSILON;
export function withinErrorMargin(left: number, right: number, tolerance = Number.EPSILON) {
return Math.abs(left - right) < tolerance;
}

/**
* Tolerance for the results of accuracy issues to tolerate certain errors
*
* Why 12?
* Why is precision 12?
This is an empirical choice. Generally, choosing 12 can solve most of the 0001 and 0009 problems. e.g. floor(5,1.23) = 0.0800000000000001
why is tolerance 1e-10?
Since the value of Number.EPSILON is too small to be applicable to all floating-point precision processing, for most application scenarios, the error range of 1e-10 can tolerate common floating-point errors.
For example, =30.2 - 30 displayed as 0.2 in Excel
* @param num
* @param precision
* @param tolerance
* @returns
*/
export function stripErrorMargin(num: number, precision = 12) {
export function stripErrorMargin(num: number, precision = 12, tolerance = 1e-10) {
const stripResult = strip(num, precision);
return withinErrorMargin(num, stripResult) ? stripResult : strip(num);
return withinErrorMargin(num, stripResult, tolerance) ? stripResult : strip(num);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Test time function', () => {
const minute = NumberValueObject.create(750); // 750 minutes = 12 hours, 30 minutes
const second = NumberValueObject.create(3661); // 3661 seconds = 1 hour, 1 minute, 1 second
const result = testFunction.calculate(hour, minute, second);
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([[0.688206018518518]]); // 16:31:01
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([[0.688206018519]]); // 16:31:01
});

it('Negative values', () => {
Expand All @@ -72,7 +72,7 @@ describe('Test time function', () => {
const minute = NumberValueObject.create(45);
const second = NumberValueObject.create(30);
const result = testFunction.calculate(hour, minute, second);
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([[0.531597222222222, 0.0315972222222222, ErrorType.NUM, ErrorType.VALUE]]);
expect(stripArrayValue(transformToValue(result.getArrayValue()))).toStrictEqual([[0.531597222222, 0.0315972222222, ErrorType.NUM, ErrorType.VALUE]]);
});

it('Handling leap values in hours, minutes, and seconds', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/engine-formula/src/functions/text/len/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ export class Len extends BaseFunction {

if (text.isNumber()) {
const numberValue = text.getValue() as number;
const numberValueString = stripErrorMargin(numberValue).toString();
// Specify Number.EPSILON to not discard necessary digits in the case of non-precision errors, for example, the length of 1/3 is 17
const numberValueString = stripErrorMargin(numberValue, 12, Number.EPSILON).toString();
return NumberValueObject.create(numberValueString.length);
}

Expand Down
19 changes: 13 additions & 6 deletions packages/facade/src/apis/sheets/__tests__/f-formula.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import type { ICellData, Injector, IStyleData, Nullable } from '@univerjs/core';
import { ICommandService, IUniverInstanceService } from '@univerjs/core';
import { SetHorizontalTextAlignCommand, SetRangeValuesCommand, SetRangeValuesMutation, SetStyleCommand, SetTextWrapCommand, SetVerticalTextAlignCommand } from '@univerjs/sheets';
import { beforeEach, describe, it } from 'vitest';
import { beforeEach, describe, expect, it } from 'vitest';

import { SetFormulaCalculationNotificationMutation, SetFormulaCalculationStartMutation, SetFormulaCalculationStopMutation } from '@univerjs/engine-formula';
import { SetArrayFormulaDataMutation, SetFormulaCalculationNotificationMutation, SetFormulaCalculationResultMutation, SetFormulaCalculationStartMutation, SetFormulaCalculationStopMutation } from '@univerjs/engine-formula';
import type { FUniver } from '../../facade';
import { createFacadeTestBed } from '../../__tests__/create-test-bed';

Expand Down Expand Up @@ -55,6 +55,8 @@ describe('Test FFormula', () => {
commandService.registerCommand(SetFormulaCalculationStartMutation);
commandService.registerCommand(SetFormulaCalculationStopMutation);
commandService.registerCommand(SetFormulaCalculationNotificationMutation);
commandService.registerCommand(SetFormulaCalculationResultMutation);
commandService.registerCommand(SetArrayFormulaDataMutation);

getValueByPosition = (
startRow: number,
Expand Down Expand Up @@ -85,14 +87,19 @@ describe('Test FFormula', () => {
it('FFormula executeCalculation', () => {
const formula = univerAPI.getFormula();

formula.calculationStart(() => {
// console.log('calculation start');
formula.calculationStart((forceCalculate) => {
expect(forceCalculate).toBe(true);
});

formula.calculationEnd((allRuntimeData) => {
// console.log('calculation end', allRuntimeData);
formula.calculationProcessing((stageInfo) => {
expect(stageInfo).toBeDefined();
});

formula.calculationEnd((functionsExecutedState) => {
expect(functionsExecutedState).toBeDefined();
});

formula.executeCalculation();
formula.stopCalculation();
});
});
57 changes: 31 additions & 26 deletions packages/facade/src/apis/sheets/f-formula.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,18 @@
* limitations under the License.
*/

import type { IDisposable } from '@univerjs/core';
import { ICommandService, Inject, toDisposable } from '@univerjs/core';
import type { IAllRuntimeData, IExecutionInProgressParams } from '@univerjs/engine-formula';
import { CalculateFormulaService, SetFormulaCalculationStartMutation, SetFormulaCalculationStopMutation } from '@univerjs/engine-formula';
import type { ICommandInfo, IDisposable } from '@univerjs/core';
import { ICommandService } from '@univerjs/core';
import type { FormulaExecutedStateType, IExecutionInProgressParams, ISetFormulaCalculationNotificationMutation, ISetFormulaCalculationStartMutation } from '@univerjs/engine-formula';
import { SetFormulaCalculationNotificationMutation, SetFormulaCalculationStartMutation, SetFormulaCalculationStopMutation } from '@univerjs/engine-formula';

/**
* This interface class provides methods to modify the behavior of the operation formula.
*/
export class FFormula {
constructor(
@ICommandService private readonly _commandService: ICommandService,
@Inject(CalculateFormulaService) private readonly _calculateFormulaService: CalculateFormulaService
@ICommandService private readonly _commandService: ICommandService
) {
// empty
}

/**
Expand Down Expand Up @@ -56,39 +54,46 @@ export class FFormula {
/**
* Listening calculation starts.
*/
calculationStart(callback: () => void): IDisposable {
const subscribe = this._calculateFormulaService.executionStartListener$.subscribe(() => {
callback();
});

return toDisposable(() => {
subscribe.unsubscribe();
calculationStart(callback: (forceCalculation: boolean) => void): IDisposable {
return this._commandService.onCommandExecuted((command: ICommandInfo) => {
if (command.id === SetFormulaCalculationStartMutation.id) {
const params = command.params as ISetFormulaCalculationStartMutation;
callback(params.forceCalculation);
}
});
}

/**
* Listening calculation ends.
*/
calculationEnd(callback: (allRuntimeData: IAllRuntimeData) => void): IDisposable {
const subscribe = this._calculateFormulaService.executionCompleteListener$.subscribe((allRuntimeData) => {
callback(allRuntimeData);
});
calculationEnd(callback: (functionsExecutedState: FormulaExecutedStateType) => void): IDisposable {
return this._commandService.onCommandExecuted((command: ICommandInfo) => {
if (command.id !== SetFormulaCalculationNotificationMutation.id) {
return;
}

const params = command.params as ISetFormulaCalculationNotificationMutation;

return toDisposable(() => {
subscribe.unsubscribe();
if (params.functionsExecutedState !== undefined) {
callback(params.functionsExecutedState);
}
});
}

/**
* Listening calculation processing.
*/
calculationProcessing(callback: (params: IExecutionInProgressParams) => void): IDisposable {
const subscribe = this._calculateFormulaService.executionInProgressListener$.subscribe((params) => {
callback(params);
});
calculationProcessing(callback: (stageInfo: IExecutionInProgressParams) => void): IDisposable {
return this._commandService.onCommandExecuted((command: ICommandInfo) => {
if (command.id !== SetFormulaCalculationNotificationMutation.id) {
return;
}

return toDisposable(() => {
subscribe.unsubscribe();
const params = command.params as ISetFormulaCalculationNotificationMutation;

if (params.stageInfo !== undefined) {
callback(params.stageInfo);
}
});
}
}

0 comments on commit a927229

Please sign in to comment.