From 02b6618d8c88f278790f84c31eef042fc868c0e7 Mon Sep 17 00:00:00 2001 From: Anthony Hendrickx Date: Wed, 13 Nov 2024 13:04:00 +0100 Subject: [PATCH] [FIX] charts: non-invertible matrix returns NaN as predicted data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task Description When trying to make a trending line, almost all model use matrix computation to get the predicted data, with some matrix multiplications and inversions. When the matrixes aren't invertible the user get a traceback, which is not fine. This PR aims to fix these issues by returning an array of NaN when there is an error in the computation of the predicted dataset. Related Task closes odoo/o-spreadsheet#5202 Task: 4328743 Signed-off-by: Lucas Lefèvre (lul) --- .../chart/chart_with_axis/design_panel.ts | 2 +- src/helpers/figures/charts/chart_common.ts | 67 ++++++++++--------- tests/figures/chart/chart_plugin.test.ts | 14 ++++ 3 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/components/side_panel/chart/chart_with_axis/design_panel.ts b/src/components/side_panel/chart/chart_with_axis/design_panel.ts index 7250b36aac..c6588ce0dc 100644 --- a/src/components/side_panel/chart/chart_with_axis/design_panel.ts +++ b/src/components/side_panel/chart/chart_with_axis/design_panel.ts @@ -195,7 +195,7 @@ export class ChartWithAxisDesignPanel

extends Component case "polynomial": config = { type: "polynomial", - order: type === "linear" ? 1 : 2, + order: type === "linear" ? 1 : this.getMaxPolynomialDegree(), }; break; case "exponential": diff --git a/src/helpers/figures/charts/chart_common.ts b/src/helpers/figures/charts/chart_common.ts index 1678e18ceb..7ed05dca34 100644 --- a/src/helpers/figures/charts/chart_common.ts +++ b/src/helpers/figures/charts/chart_common.ts @@ -535,41 +535,48 @@ export function interpolateData( const labelRange = labelMax - labelMin; const normalizedLabels = labels.map((v) => (v - labelMin) / labelRange); const normalizedNewLabels = newLabels.map((v) => (v - labelMin) / labelRange); - switch (config.type) { - case "polynomial": { - const order = config.order ?? 2; - if (order === 1) { - return predictLinearValues([values], [normalizedLabels], [normalizedNewLabels], true)[0]; + try { + switch (config.type) { + case "polynomial": { + const order = config.order; + if (!order) { + return Array.from({ length: newLabels.length }, () => NaN); + } + if (order === 1) { + return predictLinearValues([values], [normalizedLabels], [normalizedNewLabels], true)[0]; + } + const coeffs = polynomialRegression(values, normalizedLabels, order, true).flat(); + return normalizedNewLabels.map((v) => evaluatePolynomial(coeffs, v, order)); } - const coeffs = polynomialRegression(values, normalizedLabels, order, true).flat(); - return normalizedNewLabels.map((v) => evaluatePolynomial(coeffs, v, order)); - } - case "exponential": { - const positiveLogValues: number[] = []; - const filteredLabels: number[] = []; - for (let i = 0; i < values.length; i++) { - if (values[i] > 0) { - positiveLogValues.push(Math.log(values[i])); - filteredLabels.push(normalizedLabels[i]); + case "exponential": { + const positiveLogValues: number[] = []; + const filteredLabels: number[] = []; + for (let i = 0; i < values.length; i++) { + if (values[i] > 0) { + positiveLogValues.push(Math.log(values[i])); + filteredLabels.push(normalizedLabels[i]); + } + } + if (!filteredLabels.length) { + return Array.from({ length: newLabels.length }, () => NaN); } + return expM( + predictLinearValues([positiveLogValues], [filteredLabels], [normalizedNewLabels], true) + )[0]; } - if (!filteredLabels.length) { - return []; + case "logarithmic": { + return predictLinearValues( + [values], + logM([normalizedLabels]), + logM([normalizedNewLabels]), + true + )[0]; } - return expM( - predictLinearValues([positiveLogValues], [filteredLabels], [normalizedNewLabels], true) - )[0]; - } - case "logarithmic": { - return predictLinearValues( - [values], - logM([normalizedLabels]), - logM([normalizedNewLabels]), - true - )[0]; + default: + return []; } - default: - return []; + } catch (e) { + return Array.from({ length: newLabels.length }, () => NaN); } } diff --git a/tests/figures/chart/chart_plugin.test.ts b/tests/figures/chart/chart_plugin.test.ts index e484be2cb6..486a5683a7 100644 --- a/tests/figures/chart/chart_plugin.test.ts +++ b/tests/figures/chart/chart_plugin.test.ts @@ -3209,6 +3209,20 @@ describe("trending line", () => { } }); + test("non-invertible matrix doesn't throw error", () => { + // prettier-ignore + setGrid(model, { + A1: "label", + A2: "0", + A3: "1", + }); + updateChart(model, "1", { + dataSets: [{ dataRange: "A1:A3", trend: { display: true, type: "polynomial", order: 2 } }], + }); + const runtime = model.getters.getChartRuntime("1") as LineChartRuntime; + expect(runtime.chartJsConfig.data.datasets[1].data.every((x) => isNaN(Number(x)))).toBeTruthy(); + }); + test("trend line works with real date values as labels", () => { setGrid(model, { B1: "1",