Skip to content

Commit

Permalink
[FIX] charts: non-invertible matrix returns NaN as predicted data
Browse files Browse the repository at this point in the history
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 #5289

Task: 4328743
X-original-commit: 02b6618
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Hendrickx Anthony (anhe) <[email protected]>
  • Loading branch information
anhe-odoo committed Dec 3, 2024
1 parent a3c357b commit 12c4cf8
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class SeriesWithAxisDesignEditor extends Component<Props, SpreadsheetChil
case "polynomial":
config = {
type: "polynomial",
order: type === "linear" ? 1 : 2,
order: type === "linear" ? 1 : this.getMaxPolynomialDegree(index),
};
break;
case "exponential":
Expand Down
73 changes: 40 additions & 33 deletions src/helpers/figures/charts/runtime/chart_data_extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,44 +339,51 @@ 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];
}
case "trailingMovingAverage": {
return getMovingAverageValues(values, config.window);
case "trailingMovingAverage": {
return getMovingAverageValues(values, config.window);
}
default:
return Array.from({ length: newLabels.length }, () => NaN);
}
default:
return [];
} catch (e) {
return Array.from({ length: newLabels.length }, () => NaN);
}
}

Expand Down
14 changes: 14 additions & 0 deletions tests/figures/chart/chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3344,6 +3344,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",
Expand Down

0 comments on commit 12c4cf8

Please sign in to comment.