Skip to content

Commit 90d1bb3

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 #7048 Task: 5001658 X-original-commit: fee8728 Signed-off-by: Rémi Rahir (rar) <rar@odoo.com> Signed-off-by: Hendrickx Anthony (anhe) <anhe@odoo.com>
1 parent 0b84fb5 commit 90d1bb3

File tree

7 files changed

+199
-53
lines changed

7 files changed

+199
-53
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/helpers.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,3 +1135,12 @@ export function isDataNonEmpty(data: FunctionResultObject | undefined): boolean
11351135
}
11361136
return true;
11371137
}
1138+
1139+
export const noValidInputErrorMessage = _t("[[FUNCTION_NAME]] has no valid input data.");
1140+
1141+
export function emptyDataErrorMessage(argName: string): string {
1142+
return _t(
1143+
"[[FUNCTION_NAME]] expects the provided values of %(argName)s to be a non-empty matrix.",
1144+
{ argName }
1145+
);
1146+
}

‎src/functions/module_array.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,12 @@ export const MMULT = {
371371
const _matrix1 = toNumberMatrix(matrix1, "matrix1");
372372
const _matrix2 = toNumberMatrix(matrix2, "matrix2");
373373

374+
if (_matrix1.length === 0 || _matrix2.length === 0) {
375+
return new EvaluationError(
376+
_t("The first and second arguments of [[FUNCTION_NAME]] must be non-empty matrices.")
377+
);
378+
}
379+
374380
if (_matrix1.length !== _matrix2[0].length) {
375381
return new EvaluationError(
376382
_t(

‎src/functions/module_logical.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
applyVectorization,
99
conditionalVisitBoolean,
1010
isEvaluationError,
11+
noValidInputErrorMessage,
1112
toBoolean,
1213
valueNotAvailable,
1314
} from "./helpers";
@@ -32,7 +33,7 @@ export const AND = {
3233
compute: function (...logicalExpressions: Arg[]) {
3334
const { result, foundBoolean } = boolAnd(logicalExpressions);
3435
if (!foundBoolean) {
35-
return new EvaluationError(_t("[[FUNCTION_NAME]] has no valid input data."));
36+
return new EvaluationError(noValidInputErrorMessage);
3637
}
3738
return result;
3839
},
@@ -208,7 +209,7 @@ export const OR = {
208209
compute: function (...logicalExpressions: Arg[]) {
209210
const { result, foundBoolean } = boolOr(logicalExpressions);
210211
if (!foundBoolean) {
211-
return new EvaluationError(_t("[[FUNCTION_NAME]] has no valid input data."));
212+
return new EvaluationError(noValidInputErrorMessage);
212213
}
213214
return result;
214215
},
@@ -300,7 +301,7 @@ export const XOR = {
300301
return true; // no stop condition
301302
});
302303
if (!foundBoolean) {
303-
return new EvaluationError(_t("[[FUNCTION_NAME]] has no valid input data."));
304+
return new EvaluationError(noValidInputErrorMessage);
304305
}
305306
return acc;
306307
},

‎src/functions/module_statistical.ts‎

Lines changed: 61 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ import {
2929
} from "./helper_statistical";
3030
import {
3131
dichotomicSearch,
32+
emptyDataErrorMessage,
3233
inferFormat,
3334
matrixMap,
35+
noValidInputErrorMessage,
3436
reduceNumbers,
3537
reduceNumbersTextAs0,
3638
toBoolean,
@@ -158,7 +160,7 @@ function centile(
158160
count++;
159161
}
160162
});
161-
assert(count !== 0, _t("[[FUNCTION_NAME]] has no valid input data."));
163+
assert(count !== 0, noValidInputErrorMessage);
162164

163165
if (!isInclusive) {
164166
// 2nd argument must be between 1/(n+1) and n/(n+1) with n the number of data
@@ -542,8 +544,12 @@ export const FORECAST: AddFunctionDescription = {
542544
x: Arg,
543545
dataY: Matrix<FunctionResultObject>,
544546
dataX: Matrix<FunctionResultObject>
545-
): number | Matrix<number> {
547+
) {
546548
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
549+
if (flatDataX.length === 0 || flatDataY.length === 0) {
550+
return new NotAvailableError(noValidInputErrorMessage);
551+
}
552+
547553
return predictLinearValues(
548554
[flatDataY],
549555
[flatDataX],
@@ -586,7 +592,10 @@ export const GROWTH: AddFunctionDescription = {
586592
knownDataX: Matrix<FunctionResultObject> = [[]],
587593
newDataX: Matrix<FunctionResultObject> = [[]],
588594
b: Maybe<FunctionResultObject> = { value: true }
589-
): Matrix<number> {
595+
) {
596+
if (knownDataY.length === 0 || knownDataY[0].length === 0) {
597+
return new EvaluationError(emptyDataErrorMessage("known_data_y"));
598+
}
590599
return expM(
591600
predictLinearValues(
592601
logM(toNumberMatrix(knownDataY, "the first argument (known_data_y)")),
@@ -613,11 +622,11 @@ export const INTERCEPT: AddFunctionDescription = {
613622
_t("The range representing the array or matrix of independent data.")
614623
),
615624
],
616-
compute: function (
617-
dataY: Matrix<FunctionResultObject>,
618-
dataX: Matrix<FunctionResultObject>
619-
): number {
625+
compute: function (dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>) {
620626
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
627+
if (flatDataX.length === 0 || flatDataY.length === 0) {
628+
return new NotAvailableError(noValidInputErrorMessage);
629+
}
621630
const [[], [intercept]] = fullLinearRegression([flatDataX], [flatDataY]);
622631
return intercept as number;
623632
},
@@ -658,7 +667,7 @@ export const LARGE = {
658667
});
659668
const result = largests.shift();
660669
if (result === undefined) {
661-
return new EvaluationError(_t("[[FUNCTION_NAME]] has no valid input data."));
670+
return new EvaluationError(noValidInputErrorMessage);
662671
}
663672
if (count < _n) {
664673
return new EvaluationError(
@@ -702,7 +711,10 @@ export const LINEST: AddFunctionDescription = {
702711
dataX: Matrix<FunctionResultObject> = [[]],
703712
calculateB: Maybe<FunctionResultObject> = { value: true },
704713
verbose: Maybe<FunctionResultObject> = { value: false }
705-
): (number | string)[][] {
714+
) {
715+
if (dataY.length === 0 || dataY[0].length === 0) {
716+
return new EvaluationError(emptyDataErrorMessage("data_y"));
717+
}
706718
return fullLinearRegression(
707719
toNumberMatrix(dataX, "the first argument (data_y)"),
708720
toNumberMatrix(dataY, "the second argument (data_x)"),
@@ -745,7 +757,10 @@ export const LOGEST: AddFunctionDescription = {
745757
dataX: Matrix<FunctionResultObject> = [[]],
746758
calculateB: Maybe<FunctionResultObject> = { value: true },
747759
verbose: Maybe<FunctionResultObject> = { value: false }
748-
): (number | string)[][] {
760+
) {
761+
if (dataY.length === 0 || dataY[0].length === 0) {
762+
return new EvaluationError(emptyDataErrorMessage("data_y"));
763+
}
749764
const coeffs = fullLinearRegression(
750765
toNumberMatrix(dataX, "the second argument (data_x)"),
751766
logM(toNumberMatrix(dataY, "the first argument (data_y)")),
@@ -773,10 +788,8 @@ export const MATTHEWS: AddFunctionDescription = {
773788
const flatX = dataX.flat();
774789
const flatY = dataY.flat();
775790
assertSameNumberOfElements(flatX, flatY);
776-
if (flatX.length === 0) {
777-
return new EvaluationError(
778-
_t("[[FUNCTION_NAME]] expects non-empty ranges for both parameters.")
779-
);
791+
if (flatX.length === 0 || flatY.length === 0) {
792+
return new NotAvailableError(noValidInputErrorMessage);
780793
}
781794
const n = flatX.length;
782795

@@ -1022,17 +1035,10 @@ export const MINIFS = {
10221035
// -----------------------------------------------------------------------------
10231036
// PEARSON
10241037
// -----------------------------------------------------------------------------
1025-
function pearson(dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>): number {
1038+
function pearson(dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>) {
10261039
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1027-
if (flatDataX.length === 0) {
1028-
throw new EvaluationError(
1029-
_t("[[FUNCTION_NAME]] expects non-empty ranges for both parameters.")
1030-
);
1031-
}
1032-
if (flatDataX.length < 2) {
1033-
throw new EvaluationError(
1034-
_t("[[FUNCTION_NAME]] needs at least two values for both parameters.")
1035-
);
1040+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1041+
return new NotAvailableError(noValidInputErrorMessage);
10361042
}
10371043
const n = flatDataX.length;
10381044

@@ -1072,7 +1078,7 @@ export const PEARSON: AddFunctionDescription = {
10721078
compute: function (
10731079
dataY: Matrix<FunctionResultObject>,
10741080
dataX: Matrix<FunctionResultObject>
1075-
): number {
1081+
): number | NotAvailableError {
10761082
return pearson(dataY, dataX);
10771083
},
10781084
isExported: true,
@@ -1169,8 +1175,11 @@ export const POLYFIT_COEFFS: AddFunctionDescription = {
11691175
dataX: Matrix<FunctionResultObject>,
11701176
order: Maybe<FunctionResultObject>,
11711177
intercept: Maybe<FunctionResultObject> = { value: true }
1172-
): Matrix<number> {
1178+
) {
11731179
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1180+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1181+
return new NotAvailableError(noValidInputErrorMessage);
1182+
}
11741183
return polynomialRegression(
11751184
flatDataY,
11761185
flatDataX,
@@ -1208,9 +1217,12 @@ export const POLYFIT_FORECAST: AddFunctionDescription = {
12081217
dataX: Matrix<FunctionResultObject>,
12091218
order: Maybe<FunctionResultObject>,
12101219
intercept: Maybe<FunctionResultObject> = { value: true }
1211-
): Matrix<number> {
1220+
) {
12121221
const _order = toNumber(order, this.locale);
12131222
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1223+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1224+
return new NotAvailableError(noValidInputErrorMessage);
1225+
}
12141226
const coeffs = polynomialRegression(flatDataY, flatDataX, _order, toBoolean(intercept)).flat();
12151227
return matrixMap(toMatrix(x), (xij) =>
12161228
evaluatePolynomial(coeffs, toNumber(xij, this.locale), _order)
@@ -1336,7 +1348,11 @@ export const RSQ: AddFunctionDescription = {
13361348
dataY: Matrix<FunctionResultObject>,
13371349
dataX: Matrix<FunctionResultObject>
13381350
): number {
1339-
return Math.pow(pearson(dataX, dataY), 2.0);
1351+
const value = pearson(dataY, dataX);
1352+
if (value instanceof Error) {
1353+
throw value;
1354+
}
1355+
return Math.pow(value as number, 2.0);
13401356
},
13411357
isExported: true,
13421358
};
@@ -1356,11 +1372,11 @@ export const SLOPE: AddFunctionDescription = {
13561372
_t("The range representing the array or matrix of independent data.")
13571373
),
13581374
],
1359-
compute: function (
1360-
dataY: Matrix<FunctionResultObject>,
1361-
dataX: Matrix<FunctionResultObject>
1362-
): number {
1375+
compute: function (dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>) {
13631376
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1377+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1378+
return new NotAvailableError(noValidInputErrorMessage);
1379+
}
13641380
const [[slope]] = fullLinearRegression([flatDataX], [flatDataY]);
13651381
return slope as number;
13661382
},
@@ -1401,7 +1417,7 @@ export const SMALL = {
14011417
});
14021418
const result = largests.pop();
14031419
if (result === undefined) {
1404-
return new EvaluationError(_t("[[FUNCTION_NAME]] has no valid input data."));
1420+
return new EvaluationError(noValidInputErrorMessage);
14051421
}
14061422
if (count < _n) {
14071423
return new EvaluationError(
@@ -1428,11 +1444,11 @@ export const SPEARMAN: AddFunctionDescription = {
14281444
_t("The range representing the array or matrix of independent data.")
14291445
),
14301446
],
1431-
compute: function (
1432-
dataX: Matrix<FunctionResultObject>,
1433-
dataY: Matrix<FunctionResultObject>
1434-
): number {
1447+
compute: function (dataX: Matrix<FunctionResultObject>, dataY: Matrix<FunctionResultObject>) {
14351448
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1449+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1450+
return new NotAvailableError(noValidInputErrorMessage);
1451+
}
14361452
const n = flatDataX.length;
14371453

14381454
const order = flatDataX.map((e, i) => [e, flatDataY[i]]);
@@ -1577,11 +1593,11 @@ export const STEYX: AddFunctionDescription = {
15771593
_t("The range representing the array or matrix of independent data.")
15781594
),
15791595
],
1580-
compute: function (
1581-
dataY: Matrix<FunctionResultObject>,
1582-
dataX: Matrix<FunctionResultObject>
1583-
): number {
1596+
compute: function (dataY: Matrix<FunctionResultObject>, dataX: Matrix<FunctionResultObject>) {
15841597
const { flatDataX, flatDataY } = filterAndFlatData(dataY, dataX);
1598+
if (flatDataX.length === 0 || flatDataY.length === 0) {
1599+
return new NotAvailableError(noValidInputErrorMessage);
1600+
}
15851601
const data = fullLinearRegression([flatDataX], [flatDataY], true, true);
15861602
return data[1][2] as number;
15871603
},
@@ -1620,7 +1636,10 @@ export const TREND: AddFunctionDescription = {
16201636
knownDataX: Matrix<FunctionResultObject> = [[]],
16211637
newDataX: Matrix<FunctionResultObject> = [[]],
16221638
b: Maybe<FunctionResultObject> = { value: true }
1623-
): Matrix<number> {
1639+
) {
1640+
if (knownDataY.length === 0 || knownDataY[0].length === 0) {
1641+
return new EvaluationError(emptyDataErrorMessage("known_data_y"));
1642+
}
16241643
return predictLinearValues(
16251644
toNumberMatrix(knownDataY, "the first argument (known_data_y)"),
16261645
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",

0 commit comments

Comments
 (0)