From 4bbb8167f14c3a0572622ca2cc438290613e4ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Thu, 1 Aug 2019 16:07:33 +0200 Subject: [PATCH] [poincare/matrix_inverse] Handle memory error in MatrixInverse --- apps/calculation/calculation.cpp | 3 +- poincare/include/poincare/matrix.h | 2 +- poincare/src/matrix.cpp | 65 ++++++++++++++++++------------ poincare/src/matrix_inverse.cpp | 12 ++++-- 4 files changed, 52 insertions(+), 30 deletions(-) diff --git a/apps/calculation/calculation.cpp b/apps/calculation/calculation.cpp index e9af5d91c..504e74d1e 100644 --- a/apps/calculation/calculation.cpp +++ b/apps/calculation/calculation.cpp @@ -163,7 +163,8 @@ Calculation::EqualSign Calculation::exactAndApproximateDisplayedOutputsAreEqual( * checkpoint: if there was not enough memory on the pool to compute the equal * sign, just return EqualSign::Approximation. * We can safely use an exception checkpoint here because we are sure of not - * modifying any pre-existing node in the pool. */ + * modifying any pre-existing node in the pool. We are sure there cannot be a + * Store in the exactOutput. */ Poincare::ExceptionCheckpoint ecp; if (ExceptionRun(ecp)) { constexpr int bufferSize = Constant::MaxSerializedExpressionSize; diff --git a/poincare/include/poincare/matrix.h b/poincare/include/poincare/matrix.h index beb661f87..c5ade6f1f 100644 --- a/poincare/include/poincare/matrix.h +++ b/poincare/include/poincare/matrix.h @@ -80,7 +80,7 @@ public: Matrix createTranspose() const; /* createInverse can be called on any matrix, reduced or not, approximated or * not. */ - Expression createInverse(ExpressionNode::ReductionContext reductionContext) const; + Expression createInverse(ExpressionNode::ReductionContext reductionContext, bool * couldComputeInverse) const; #if MATRIX_EXACT_REDUCING Expression determinant() const; #endif diff --git a/poincare/src/matrix.cpp b/poincare/src/matrix.cpp index 4cdfc3cb8..0e4961aa8 100644 --- a/poincare/src/matrix.cpp +++ b/poincare/src/matrix.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -301,38 +302,52 @@ Matrix Matrix::createTranspose() const { return matrix; } -Expression Matrix::createInverse(ExpressionNode::ReductionContext reductionContext) const { +Expression Matrix::createInverse(ExpressionNode::ReductionContext reductionContext, bool * couldComputeInverse) const { int dim = numberOfRows(); if (dim != numberOfColumns()) { return Undefined::Builder(); } - /* Create the matrix inv = (A|I) with A is the input matrix and I the dim - * identity matrix */ - Matrix matrixAI = Matrix::Builder(); - for (int i = 0; i < dim; i++) { - for (int j = 0; j < dim; j++) { - matrixAI.addChildAtIndexInPlace(const_cast(this)->matrixChild(i, j).clone(), i*2*dim+j, i*2*dim+j); + /* If the matrix is too big, the inversion might not be computed exactly + * because of a pool allocation error, but we might still be able to compute + * it approximately. We thus encapsulate the inversion creation in an + * exception checkpoint. + * We can safely use an exception checkpoint here because we are sure of not + * modifying any pre-existing node in the pool. We are sure there is no Store + * in the matrix. */ + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { + /* Create the matrix inv = (A|I) with A is the input matrix and I the dim + * identity matrix */ + Matrix matrixAI = Matrix::Builder(); + for (int i = 0; i < dim; i++) { + for (int j = 0; j < dim; j++) { + matrixAI.addChildAtIndexInPlace(const_cast(this)->matrixChild(i, j).clone(), i*2*dim+j, i*2*dim+j); + } + for (int j = dim; j < 2*dim; j++) { + matrixAI.addChildAtIndexInPlace(j-dim == i ? Rational::Builder(1) : Rational::Builder(0), i*2*dim+j, i*2*dim+j); + } } - for (int j = dim; j < 2*dim; j++) { - matrixAI.addChildAtIndexInPlace(j-dim == i ? Rational::Builder(1) : Rational::Builder(0), i*2*dim+j, i*2*dim+j); + matrixAI.setDimensions(dim, 2*dim); + matrixAI = matrixAI.rowCanonize(reductionContext); + // Check inversibility + for (int i = 0; i < dim; i++) { + if (!matrixAI.matrixChild(i, i).isRationalOne()) { + return Undefined::Builder(); + } } + Matrix inverse = Matrix::Builder(); + for (int i = 0; i < dim; i++) { + for (int j = 0; j < dim; j++) { + inverse.addChildAtIndexInPlace(matrixAI.matrixChild(i, j+dim), i*dim+j, i*dim+j); // We can steal matrixAI's children + } + } + inverse.setDimensions(dim, dim); + *couldComputeInverse = true; + return inverse; + } else { + *couldComputeInverse = false; + return Expression(); } - matrixAI.setDimensions(dim, 2*dim); - matrixAI = matrixAI.rowCanonize(reductionContext); - // Check inversibility - for (int i = 0; i < dim; i++) { - if (!matrixAI.matrixChild(i, i).isRationalOne()) { - return Undefined::Builder(); - } - } - Matrix inverse = Matrix::Builder(); - for (int i = 0; i < dim; i++) { - for (int j = 0; j < dim; j++) { - inverse.addChildAtIndexInPlace(matrixAI.matrixChild(i, j+dim), i*dim+j, i*dim+j); // We can steal matrixAI's children - } - } - inverse.setDimensions(dim, dim); - return inverse; } template int Matrix::ArrayInverse(float *, int, int); diff --git a/poincare/src/matrix_inverse.cpp b/poincare/src/matrix_inverse.cpp index 085aeed63..2590626ff 100644 --- a/poincare/src/matrix_inverse.cpp +++ b/poincare/src/matrix_inverse.cpp @@ -57,9 +57,15 @@ Expression MatrixInverse::shallowReduce(ExpressionNode::ReductionContext reducti } /* Power(matrix, -n) creates a matrixInverse, so the simplification must be * done here and not in power. */ - Expression result = matrixChild.createInverse(reductionContext); - replaceWithInPlace(result); - return result.shallowReduce(reductionContext); + bool couldComputeInverse = false; + Expression result = matrixChild.createInverse(reductionContext, &couldComputeInverse); + if (couldComputeInverse) { + replaceWithInPlace(result); + return result.shallowReduce(reductionContext); + } + // The matrix could not be inverted exactly + // TODO Poincare error? + return *this; } Expression result = Power::Builder(c, Rational::Builder(-1)); replaceWithInPlace(result);