Skip to content

Commit f3f5850

Browse files
anhe-odooanhe-odoo
committed
[FIX] evaluation: fix operation with empty matrices
Task Description When using empty matrices for math operation (multiplication or inversion), as in the FORECAST formula, we got a traceback and the error message was not clear for the user. This commits aims to fix it by checking that the matrices used in theses operations are not empty and returning an understandable error if it's not the case. Related Task closes #7034 Task: 5001658 X-original-commit: 867ce19 Signed-off-by: Rémi Rahir (rar) <rar@odoo.com> Signed-off-by: Hendrickx Anthony (anhe) <anhe@odoo.com>
1 parent 14a6da6 commit f3f5850

File tree

6 files changed

+157
-25
lines changed

6 files changed

+157
-25
lines changed

‎src/functions/helper_matrices.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import { _t } from "../translation";
21
import { Matrix, isMatrix } from "../types";
3-
import { EvaluationError } from "../types/errors";
42

53
export function getUnitMatrix(n: number): Matrix<number> {
64
const matrix: Matrix<number> = Array(n);
@@ -31,10 +29,12 @@ export function invertMatrix(M: Matrix<number>): {
3129
// (b) Multiply a row by a scalar. This multiply the determinant by that scalar.
3230
// (c) Add to a row a multiple of another row. This does not change the determinant.
3331

32+
if (M.length < 1 || M[0].length < 1) {
33+
throw new Error("invertMatrix: an empty matrix cannot be inverted.");
34+
}
35+
3436
if (M.length !== M[0].length) {
35-
throw new EvaluationError(
36-
_t("Function [[FUNCTION_NAME]] invert matrix error, only square matrices are invertible")
37-
);
37+
throw new Error("invertMatrix: only square matrices are invertible");
3838
}
3939

4040
let determinant = 1;
@@ -114,8 +114,11 @@ function swapMatrixRows(matrix: number[][], row1: number, row2: number) {
114114
* Note: we use indexing [col][row] instead of the standard mathematical notation [row][col]
115115
*/
116116
export function multiplyMatrices(matrix1: Matrix<number>, matrix2: Matrix<number>): Matrix<number> {
117+
if (matrix1.length < 1 || matrix2.length < 1) {
118+
throw new Error("multiplyMatrices: empty matrices cannot be multiplied.");
119+
}
117120
if (matrix1.length !== matrix2[0].length) {
118-
throw new EvaluationError(_t("Cannot multiply matrices : incompatible matrices size."));
121+
throw new Error("multiplyMatrices: incompatible matrices size.");
119122
}
120123

121124
const rowsM1 = matrix1[0].length;
@@ -144,7 +147,7 @@ export function toScalar<T>(arg: Matrix<T> | T): T {
144147
return arg;
145148
}
146149
if (!isSingleElementMatrix(arg)) {
147-
throw new EvaluationError(_t("The value should be a scalar or a 1x1 matrix"));
150+
throw new Error("The value should be a scalar or a 1x1 matrix");
148151
}
149152
return arg[0][0];
150153
}

‎src/functions/helper_statistical.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { isNumber, parseDateTime, range } from "../helpers";
22
import { _t } from "../translation";
33
import { Arg, Locale, Matrix, isMatrix } from "../types";
4-
import { EvaluationError } from "../types/errors";
4+
import { EvaluationError, NotAvailableError } from "../types/errors";
55
import { invertMatrix, multiplyMatrices } from "./helper_matrices";
66
import {
77
assert,
@@ -284,3 +284,18 @@ export function predictLinearValues(
284284
});
285285
return newY.length === newX.length ? newY : transposeMatrix(newY);
286286
}
287+
288+
export function assertNonEmptyMatrix(matrix: any[][], argName: string) {
289+
assert(
290+
() => matrix.length > 0 && matrix[0].length > 0,
291+
_t("[[FUNCTION_NAME]] expects the provided values of %(argName)s to be a non-empty matrix.", {
292+
argName,
293+
})
294+
);
295+
}
296+
297+
export function assertNonEmpty(...data: any[][]) {
298+
if (data.length === 0 || data.some((arg) => arg.length === 0)) {
299+
throw new NotAvailableError(_t("[[FUNCTION_NAME]] has no valid input data."));
300+
}
301+
}

‎src/functions/module_array.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,11 @@ export const MMULT = {
368368
const _matrix1 = toNumberMatrix(matrix1, "matrix1");
369369
const _matrix2 = toNumberMatrix(matrix2, "matrix2");
370370

371+
assert(
372+
() => _matrix1.length > 0 && _matrix2.length > 0,
373+
_t("The first and second arguments of [[FUNCTION_NAME]] must be non-empty matrices.")
374+
);
375+
371376
assert(
372377
() => _matrix1.length === _matrix2[0].length,
373378
_t(

‎src/functions/module_statistical.ts‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import {
1010
Maybe,
1111
isMatrix,
1212
} from "../types";
13-
import { CellErrorType, EvaluationError, NotAvailableError } from "../types/errors";
13+
import { CellErrorType, NotAvailableError } from "../types/errors";
1414
import { arg } from "./arguments";
1515
import { assertSameDimensions } from "./helper_assert";
1616
import {
17+
assertNonEmpty,
18+
assertNonEmptyMatrix,
1719
assertSameNumberOfElements,
1820
average,
1921
countAny,
@@ -524,6 +526,8 @@ export const FORECAST: AddFunctionDescription = {
524526
dataX: Matrix<FunctionResultObject>
525527
): number | Matrix<number> {
526528
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
529+
assertNonEmpty(flatDataX, flatDataY);
530+
527531
return predictLinearValues(
528532
[flatDataY],
529533
[flatDataX],
@@ -567,6 +571,7 @@ export const GROWTH: AddFunctionDescription = {
567571
newDataX: Matrix<FunctionResultObject> = [[]],
568572
b: Maybe<FunctionResultObject> = { value: true }
569573
): Matrix<number> {
574+
assertNonEmptyMatrix(knownDataY, "known_data_y");
570575
return expM(
571576
predictLinearValues(
572577
logM(toNumberMatrix(knownDataY, "the first argument (known_data_y)")),
@@ -598,6 +603,7 @@ export const INTERCEPT: AddFunctionDescription = {
598603
dataX: Matrix<FunctionResultObject>
599604
): number {
600605
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
606+
assertNonEmpty(flatDataX, flatDataY);
601607
const [[], [intercept]] = fullLinearRegression([flatDataX], [flatDataY]);
602608
return intercept as number;
603609
},
@@ -680,6 +686,7 @@ export const LINEST: AddFunctionDescription = {
680686
calculateB: Maybe<FunctionResultObject> = { value: true },
681687
verbose: Maybe<FunctionResultObject> = { value: false }
682688
): (number | string)[][] {
689+
assertNonEmptyMatrix(dataY, "data_y");
683690
return fullLinearRegression(
684691
toNumberMatrix(dataX, "the first argument (data_y)"),
685692
toNumberMatrix(dataY, "the second argument (data_x)"),
@@ -723,6 +730,7 @@ export const LOGEST: AddFunctionDescription = {
723730
calculateB: Maybe<FunctionResultObject> = { value: true },
724731
verbose: Maybe<FunctionResultObject> = { value: false }
725732
): (number | string)[][] {
733+
assertNonEmptyMatrix(dataY, "data_y");
726734
const coeffs = fullLinearRegression(
727735
toNumberMatrix(dataX, "the second argument (data_x)"),
728736
logM(toNumberMatrix(dataY, "the first argument (data_y)")),
@@ -750,11 +758,7 @@ export const MATTHEWS: AddFunctionDescription = {
750758
const flatX = dataX.flat();
751759
const flatY = dataY.flat();
752760
assertSameNumberOfElements(flatX, flatY);
753-
if (flatX.length === 0) {
754-
return new EvaluationError(
755-
_t("[[FUNCTION_NAME]] expects non-empty ranges for both parameters.")
756-
);
757-
}
761+
assertNonEmpty(flatX, flatY);
758762
const n = flatX.length;
759763

760764
let trueN = 0,
@@ -1001,16 +1005,7 @@ export const MINIFS = {
10011005
// -----------------------------------------------------------------------------
10021006
function pearson(dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>): number {
10031007
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1004-
if (flatDataX.length === 0) {
1005-
throw new EvaluationError(
1006-
_t("[[FUNCTION_NAME]] expects non-empty ranges for both parameters.")
1007-
);
1008-
}
1009-
if (flatDataX.length < 2) {
1010-
throw new EvaluationError(
1011-
_t("[[FUNCTION_NAME]] needs at least two values for both parameters.")
1012-
);
1013-
}
1008+
assertNonEmpty(flatDataX, flatDataY);
10141009
const n = flatDataX.length;
10151010

10161011
let sumX = 0,
@@ -1148,6 +1143,7 @@ export const POLYFIT_COEFFS: AddFunctionDescription = {
11481143
intercept: Maybe<FunctionResultObject> = { value: true }
11491144
): Matrix<number> {
11501145
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1146+
assertNonEmpty(flatDataX, flatDataY);
11511147
return polynomialRegression(
11521148
flatDataY,
11531149
flatDataX,
@@ -1188,6 +1184,7 @@ export const POLYFIT_FORECAST: AddFunctionDescription = {
11881184
): Matrix<number> {
11891185
const _order = toNumber(order, this.locale);
11901186
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1187+
assertNonEmpty(flatDataX, flatDataY);
11911188
const coeffs = polynomialRegression(flatDataY, flatDataX, _order, toBoolean(intercept)).flat();
11921189
return matrixMap(toMatrix(x), (xij) =>
11931190
evaluatePolynomial(coeffs, toNumber(xij, this.locale), _order)
@@ -1338,6 +1335,7 @@ export const SLOPE: AddFunctionDescription = {
13381335
dataX: Matrix<FunctionResultObject>
13391336
): number {
13401337
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1338+
assertNonEmpty(flatDataX, flatDataY);
13411339
const [[slope]] = fullLinearRegression([flatDataX], [flatDataY]);
13421340
return slope as number;
13431341
},
@@ -1407,6 +1405,7 @@ export const SPEARMAN: AddFunctionDescription = {
14071405
dataY: Matrix<FunctionResultObject>
14081406
): number {
14091407
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1408+
assertNonEmpty(flatDataX, flatDataY);
14101409
const n = flatDataX.length;
14111410

14121411
const order = flatDataX.map((e, i) => [e, flatDataY[i]]);
@@ -1556,6 +1555,7 @@ export const STEYX: AddFunctionDescription = {
15561555
dataX: Matrix<FunctionResultObject>
15571556
): number {
15581557
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1558+
assertNonEmpty(flatDataX, flatDataY);
15591559
const data = fullLinearRegression([flatDataX], [flatDataY], true, true);
15601560
return data[1][2] as number;
15611561
},
@@ -1595,6 +1595,7 @@ export const TREND: AddFunctionDescription = {
15951595
newDataX: Matrix<FunctionResultObject> = [[]],
15961596
b: Maybe<FunctionResultObject> = { value: true }
15971597
): Matrix<number> {
1598+
assertNonEmptyMatrix(knownDataY, "known_data_y");
15981599
return predictLinearValues(
15991600
toNumberMatrix(knownDataY, "the first argument (known_data_y)"),
16001601
toNumberMatrix(knownDataX, "the second argument (known_data_x)"),

‎tests/functions/module_array.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Model } from "../../src";
2+
import { ErrorCell } from "../../src/types";
23
import { setCellContent, setFormat } from "../test_helpers/commands_helpers";
34
import { getCellContent, getEvaluatedCell, getRangeValues } from "../test_helpers/getters_helpers";
45
import {
@@ -822,6 +823,15 @@ describe("MDETERM function", () => {
822823
};
823824
expect(evaluateCell("D1", { D1: "=MDETERM(A1:C3)", ...grid })).toBeCloseTo(-4885.9);
824825
});
826+
827+
test("Determinant of an empty matrix", () => {
828+
const model = createModelFromGrid({});
829+
setCellContent(model, "D1", "=MDETERM(A1:C3)");
830+
expect(getEvaluatedCell(model, "D1").value).toBe("#ERROR");
831+
expect((getEvaluatedCell(model, "D1") as ErrorCell).message).toBe(
832+
"Function MDETERM expects number values for square_matrix, but got a object."
833+
);
834+
});
825835
});
826836

827837
describe("MINVERSE function", () => {
@@ -856,6 +866,15 @@ describe("MINVERSE function", () => {
856866
expect(evaluateCell("D1", { D1: "=MINVERSE(5)", ...grid })).toEqual(1 / 5);
857867
});
858868

869+
test("Inverse of an empty matrix", () => {
870+
const model = createModelFromGrid({});
871+
setCellContent(model, "D1", "=MINVERSE(A1:C3)");
872+
expect(getEvaluatedCell(model, "D1").value).toBe("#ERROR");
873+
expect((getEvaluatedCell(model, "D1") as ErrorCell).message).toBe(
874+
"Function MINVERSE expects number values for square_matrix, but got a object."
875+
);
876+
});
877+
859878
test("Invert matrices", () => {
860879
//prettier-ignore
861880
let grid = {
@@ -914,7 +933,7 @@ describe("MMULT function", () => {
914933
expect(evaluateCell("D1", { D1: "=MMULT(5, 5)", ...grid })).toEqual(25);
915934
});
916935

917-
test("Invert matrices", () => {
936+
test("Multiply matrices", () => {
918937
//prettier-ignore
919938
const grid = {
920939
A1: "1", B1: "2", C1: "3",

‎tests/functions/module_statistical.test.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { toXC } from "../../src/helpers";
2+
import { ErrorCell } from "../../src/types";
3+
import { setCellContent } from "../test_helpers/commands_helpers";
24
import { getEvaluatedCell } from "../test_helpers/getters_helpers";
35
import {
46
createModelFromGrid,
@@ -3185,6 +3187,23 @@ describe("MATTHEWS formula", () => {
31853187
});
31863188

31873189
describe("SLOPE formula", () => {
3190+
test("Slope with an empty matrix for the y values", () => {
3191+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3192+
setCellContent(model, "A1", "=SLOPE(B2:B3, C2:C3)");
3193+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3194+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3195+
"SLOPE has no valid input data."
3196+
);
3197+
});
3198+
3199+
test("Slope with an empty matrix for the x values", () => {
3200+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3201+
setCellContent(model, "A1", "=SLOPE(C2:C3, B2:B3)");
3202+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3203+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3204+
"SLOPE has no valid input data."
3205+
);
3206+
});
31883207
test("Unrelated values", () => {
31893208
//prettier-ignore
31903209
const grid = {
@@ -3246,6 +3265,23 @@ describe("SLOPE formula", () => {
32463265
});
32473266

32483267
describe("INTERCEPT formula", () => {
3268+
test("Intercept with an empty matrix for the y values", () => {
3269+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3270+
setCellContent(model, "A1", "=INTERCEPT(B2:B3, C2:C3)");
3271+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3272+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3273+
"INTERCEPT has no valid input data."
3274+
);
3275+
});
3276+
3277+
test("Intercept with an empty matrix for the x values", () => {
3278+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3279+
setCellContent(model, "A1", "=INTERCEPT(C2:C3, B2:B3)");
3280+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3281+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3282+
"INTERCEPT has no valid input data."
3283+
);
3284+
});
32493285
test("Unrelated values", () => {
32503286
//prettier-ignore
32513287
const grid = {
@@ -3307,6 +3343,24 @@ describe("INTERCEPT formula", () => {
33073343
});
33083344

33093345
describe("FORECAST formula", () => {
3346+
test("Forecast with an empty matrix for the y values", () => {
3347+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3348+
setCellContent(model, "A1", "=FORECAST(1, B2:B3, C2:C3)");
3349+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3350+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3351+
"FORECAST has no valid input data."
3352+
);
3353+
});
3354+
3355+
test("Forecast with an empty matrix for the x values", () => {
3356+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3357+
setCellContent(model, "A1", "=FORECAST(1, C2:C3, B2:B3)");
3358+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3359+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3360+
"FORECAST has no valid input data."
3361+
);
3362+
});
3363+
33103364
test("Correctly predicts a single value", () => {
33113365
//prettier-ignore
33123366
const grid = {
@@ -3434,6 +3488,23 @@ describe("STEYX formula", () => {
34343488
});
34353489

34363490
describe("POLYFIT.COEFFS formula", () => {
3491+
test("Empty matrix for the y values", () => {
3492+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3493+
setCellContent(model, "A1", "=POLYFIT.COEFFS(B2:B3, C2:C3, 2)");
3494+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3495+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3496+
"POLYFIT.COEFFS has no valid input data."
3497+
);
3498+
});
3499+
3500+
test("Empty matrix for the x values", () => {
3501+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3502+
setCellContent(model, "A1", "=POLYFIT.COEFFS(C2:C3, B2:B3, 2)");
3503+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3504+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3505+
"POLYFIT.COEFFS has no valid input data."
3506+
);
3507+
});
34373508
test("Noisy values", () => {
34383509
//prettier-ignore
34393510
const grid = {
@@ -3508,6 +3579,24 @@ describe("POLYFIT.COEFFS formula", () => {
35083579
});
35093580

35103581
describe("POLYFIT.FORECAST formula", () => {
3582+
test("Empty matrix for the y values", () => {
3583+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3584+
setCellContent(model, "A1", "=POLYFIT.FORECAST(1, B2:B3, C2:C3, 2)");
3585+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3586+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3587+
"POLYFIT.FORECAST has no valid input data."
3588+
);
3589+
});
3590+
3591+
test("Empty matrix for the x values", () => {
3592+
const model = createModelFromGrid({ B2: "", B3: "", C2: "1", C3: "2" });
3593+
setCellContent(model, "A1", "=POLYFIT.FORECAST(1, C2:C3, B2:B3, 2)");
3594+
expect(getEvaluatedCell(model, "A1").value).toBe("#N/A");
3595+
expect((getEvaluatedCell(model, "A1") as ErrorCell).message).toBe(
3596+
"POLYFIT.FORECAST has no valid input data."
3597+
);
3598+
});
3599+
35113600
test.each(["1", "2", "3", "4"])("degree %s polynomial data", async (degree: string) => {
35123601
const order = parseInt(degree);
35133602
//prettier-ignore

0 commit comments

Comments
 (0)