From 8ea8b481de07169706343f35f928eac845b706fe Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Fri, 24 Oct 2008 21:42:03 +0000 Subject: As discussed on ML: * remove the automatic resizing feature of operator = * add function Matrix::set() to be used when the previous behavior is wanted * the default constructor of dynamic-size matrices now creates a "null" matrix (data=0, rows = cols = 0) instead of a 1x1 matrix * fix UnixX typos ;) --- Eigen/src/Core/Dot.h | 1 + Eigen/src/Core/Matrix.h | 68 +++++++++++++++++++++++++++++++++++++----- Eigen/src/Core/MatrixStorage.h | 8 ++++- Eigen/src/Core/Product.h | 3 ++ Eigen/src/Core/Redux.h | 1 + Eigen/src/Core/Sum.h | 1 + doc/QuickStartGuide.dox | 6 ++-- test/basicstuff.cpp | 14 +++++++++ test/cholesky.cpp | 2 +- test/product.h | 2 +- 10 files changed, 93 insertions(+), 13 deletions(-) diff --git a/Eigen/src/Core/Dot.h b/Eigen/src/Core/Dot.h index eda266021..e9be929ce 100644 --- a/Eigen/src/Core/Dot.h +++ b/Eigen/src/Core/Dot.h @@ -153,6 +153,7 @@ struct ei_dot_impl typedef typename Derived1::Scalar Scalar; static Scalar run(const Derived1& v1, const Derived2& v2) { + ei_assert(v1.size()>0 && "you are using a non initialized vector"); Scalar res; res = v1.coeff(0) * ei_conj(v2.coeff(0)); for(int i = 1; i < v1.size(); i++) diff --git a/Eigen/src/Core/Matrix.h b/Eigen/src/Core/Matrix.h index e129c6a7e..4e24e84e2 100644 --- a/Eigen/src/Core/Matrix.h +++ b/Eigen/src/Core/Matrix.h @@ -190,6 +190,17 @@ class Matrix inline Scalar *data() { return m_storage.data(); } + /** Resizes \c *this to a \a rows x \a cols matrix. + * + * Makes sense for dynamic-size matrices only. + * + * If the current number of coefficients of \c *this exactly matches the + * product \a rows * \a cols, then no memory allocation is performed and + * the current values are left unchanged. In all other cases, including + * shrinking, the data is reallocated and all previous values are lost. + * + * \sa resize(int) for vectors. + */ inline void resize(int rows, int cols) { ei_assert(rows > 0 @@ -201,8 +212,13 @@ class Matrix m_storage.resize(rows * cols, rows, cols); } + /** Resizes \c *this to a vector of length \a size + * + * \sa resize(int,int) for the details. + */ inline void resize(int size) { + ei_assert(size>0 && "a vector cannot be resized to 0 length"); EIGEN_STATIC_ASSERT_VECTOR_ONLY(Matrix) if(RowsAtCompileTime == 1) m_storage.resize(size, 1, size); @@ -210,16 +226,36 @@ class Matrix m_storage.resize(size, size, 1); } - /** Copies the value of the expression \a other into *this. + /** Copies the value of the expression \a other into \c *this. * - * *this is resized (if possible) to match the dimensions of \a other. + * \warning Note that the sizes of \c *this and \a other must match. + * If you want automatic resizing, then you must use the function set(). * * As a special exception, copying a row-vector into a vector (and conversely) - * is allowed. The resizing, if any, is then done in the appropriate way so that - * row-vectors remain row-vectors and vectors remain vectors. + * is allowed. + * + * \sa set() */ template inline Matrix& operator=(const MatrixBase& other) + { + ei_assert(m_storage.data()!=0 && "you cannot use operator= with a non initialized matrix (instead use set()"); + return Base::operator=(other.derived()); + } + + /** Copies the value of the expression \a other into \c *this with automatic resizing. + * + * This function is the same than the assignment operator = excepted that \c *this might + * be resized to match the dimensions of \a other. + * + * Note that copying a row-vector into a vector (and conversely) is allowed. + * The resizing, if any, is then done in the appropriate way so that row-vectors + * remain row-vectors and vectors remain vectors. + * + * \sa operator=() + */ + template + inline Matrix& set(const MatrixBase& other) { if(RowsAtCompileTime == 1) { @@ -252,10 +288,28 @@ class Matrix * * For fixed-size matrices, does nothing. * - * For dynamic-size matrices, initializes with initial size 1x1, which is inefficient, hence - * when performance matters one should avoid using this constructor on dynamic-size matrices. + * For dynamic-size matrices, creates an empty matrix of size null. + * \warning while creating such an \em null matrix is allowed, it \b cannot + * \b be \b used before having being resized or initialized with the function set(). + * In particular, initializing a null matrix with operator = is not supported. + * Finally, this constructor is the unique way to create null matrices: resizing + * a matrix to 0 is not supported. + * Here are some examples: + * \code + * MatrixXf r = MatrixXf::Random(3,4); // create a random matrix of floats + * MatrixXf m1, m2; // creates two null matrices of float + * + * m1 = r; // illegal (raise an assertion) + * r = m1; // illegal (raise an assertion) + * m1 = m2; // illegal (raise an assertion) + * m1.set(r); // OK + * m2.resize(3,4); + * m2 = r; // OK + * \endcode + * + * \sa resize(int,int), set() */ - inline explicit Matrix() : m_storage(1, 1, 1) + inline explicit Matrix() : m_storage() { ei_assert(RowsAtCompileTime > 0 && ColsAtCompileTime > 0); } diff --git a/Eigen/src/Core/MatrixStorage.h b/Eigen/src/Core/MatrixStorage.h index 95fa89809..0281d8637 100644 --- a/Eigen/src/Core/MatrixStorage.h +++ b/Eigen/src/Core/MatrixStorage.h @@ -44,7 +44,7 @@ template class ei_matrix_storage { ei_aligned_array m_data; public: - inline ei_matrix_storage() {} + inline explicit ei_matrix_storage() {} inline ei_matrix_storage(int,int,int) {} inline void swap(ei_matrix_storage& other) { std::swap(m_data,other.m_data); } inline static int rows(void) {return _Rows;} @@ -61,6 +61,7 @@ template class ei_matrix_storage class ei_matrix_storage m_data; int m_rows; public: + inline explicit ei_matrix_storage() : m_rows(0) {} inline ei_matrix_storage(int, int rows, int) : m_rows(rows) {} inline ~ei_matrix_storage() {} inline void swap(ei_matrix_storage& other) { std::swap(m_data,other.m_data); std::swap(m_rows,other.m_rows); } @@ -101,6 +103,7 @@ template class ei_matrix_storage m_data; int m_cols; public: + inline explicit ei_matrix_storage() : m_cols(0) {} inline ei_matrix_storage(int, int, int cols) : m_cols(cols) {} inline ~ei_matrix_storage() {} inline void swap(ei_matrix_storage& other) { std::swap(m_data,other.m_data); std::swap(m_cols,other.m_cols); } @@ -121,6 +124,7 @@ template class ei_matrix_storage int m_rows; int m_cols; public: + inline explicit ei_matrix_storage() : m_data(0), m_rows(0), m_cols(0) {} inline ei_matrix_storage(int size, int rows, int cols) : m_data(ei_aligned_malloc(size)), m_rows(rows), m_cols(cols) {} inline ~ei_matrix_storage() { ei_aligned_free(m_data); } @@ -148,6 +152,7 @@ template class ei_matrix_storage(size)), m_cols(cols) {} inline ~ei_matrix_storage() { ei_aligned_free(m_data); } inline void swap(ei_matrix_storage& other) { std::swap(m_data,other.m_data); std::swap(m_cols,other.m_cols); } @@ -172,6 +177,7 @@ template class ei_matrix_storage(size)), m_rows(rows) {} inline ~ei_matrix_storage() { ei_aligned_free(m_data); } inline void swap(ei_matrix_storage& other) { std::swap(m_data,other.m_data); std::swap(m_rows,other.m_rows); } diff --git a/Eigen/src/Core/Product.h b/Eigen/src/Core/Product.h index 429cdc0e9..de356c6db 100644 --- a/Eigen/src/Core/Product.h +++ b/Eigen/src/Core/Product.h @@ -312,6 +312,7 @@ struct ei_product_coeff_impl { inline static void run(int row, int col, const Lhs& lhs, const Rhs& rhs, typename Lhs::Scalar& res) { + ei_assert(lhs.cols()>0 && "you are using a non initialized matrix"); res = lhs.coeff(row, 0) * rhs.coeff(0, col); for(int i = 1; i < lhs.cols(); i++) res += lhs.coeff(row, i) * rhs.coeff(i, col); @@ -469,6 +470,7 @@ struct ei_product_packet_impl0 && "you are using a non initialized matrix"); res = ei_pmul(ei_pset1(lhs.coeff(row, 0)),rhs.template packet(0, col)); for(int i = 1; i < lhs.cols(); i++) res = ei_pmadd(ei_pset1(lhs.coeff(row, i)), rhs.template packet(i, col), res); @@ -480,6 +482,7 @@ struct ei_product_packet_impl0 && "you are using a non initialized matrix"); res = ei_pmul(lhs.template packet(row, 0), ei_pset1(rhs.coeff(0, col))); for(int i = 1; i < lhs.cols(); i++) res = ei_pmadd(lhs.template packet(row, i), ei_pset1(rhs.coeff(i, col)), res); diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 6e4f9897d..041a98479 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -65,6 +65,7 @@ struct ei_redux_impl typedef typename ei_result_of::type Scalar; static Scalar run(const Derived& mat, const BinaryOp& func) { + ei_assert(mat.rows()>0 && mat.cols()>0 && "you are using a non initialized matrix"); Scalar res; res = mat.coeff(0,0); for(int i = 1; i < mat.rows(); i++) diff --git a/Eigen/src/Core/Sum.h b/Eigen/src/Core/Sum.h index 12a84e3f1..8679b2b0a 100644 --- a/Eigen/src/Core/Sum.h +++ b/Eigen/src/Core/Sum.h @@ -165,6 +165,7 @@ struct ei_sum_impl typedef typename Derived::Scalar Scalar; static Scalar run(const Derived& mat) { + ei_assert(mat.rows()>0 && mat.cols()>0 && "you are using a non initialized matrix"); Scalar res; res = mat.coeff(0, 0); for(int i = 1; i < mat.rows(); i++) diff --git a/doc/QuickStartGuide.dox b/doc/QuickStartGuide.dox index 805b840f3..1d2f623b3 100644 --- a/doc/QuickStartGuide.dox +++ b/doc/QuickStartGuide.dox @@ -156,9 +156,9 @@ x.setRandom(size); Basis vectors \link MatrixBase::Unit [details]\endlink \code -Vector3f::UnixX() // 1 0 0 -Vector3f::UnixY() // 0 1 0 -Vector3f::UnixZ() // 0 0 1 +Vector3f::UnitX() // 1 0 0 +Vector3f::UnitY() // 0 1 0 +Vector3f::UnitZ() // 0 0 1 \endcode\code VectorXf::Unit(size,i) VectorXf::Unit(4,1) == Vector4f(0,1,0,0) diff --git a/test/basicstuff.cpp b/test/basicstuff.cpp index 40e999c87..75efd4d10 100644 --- a/test/basicstuff.cpp +++ b/test/basicstuff.cpp @@ -95,6 +95,20 @@ template void basicStuff(const MatrixType& m) VERIFY_RAISES_ASSERT(m1 = (m2.block(0,0, rows-1, cols-1))); } + // test set + { + VERIFY_IS_APPROX(m3.set(m1),m1); + MatrixType m4, m5; + VERIFY_IS_APPROX(m4.set(m1),m1); + if (MatrixType::RowsAtCompileTime==Dynamic && MatrixType::ColsAtCompileTime==Dynamic) + { + MatrixType m6(rows+1,cols); + VERIFY_RAISES_ASSERT(m5 = m1); + VERIFY_RAISES_ASSERT(m3 = m5); + VERIFY_RAISES_ASSERT(m3 = m6); + } + } + // test swap m3 = m1; m1.swap(m2); diff --git a/test/cholesky.cpp b/test/cholesky.cpp index f7eb7800e..a64a45c45 100644 --- a/test/cholesky.cpp +++ b/test/cholesky.cpp @@ -65,7 +65,7 @@ template void cholesky(const MatrixType& m) Gsl::cholesky_solve(gMatA, gVecB, gVecX); VectorType vecX, _vecX, _vecB; convert(gVecX, _vecX); - vecX = symm.cholesky().solve(vecB); + vecX.set( symm.cholesky().solve(vecB) ); Gsl::prod(gSymm, gVecX, gVecB); convert(gVecB, _vecB); // test gsl itself ! diff --git a/test/product.h b/test/product.h index f1a16cf1f..5ca56d995 100644 --- a/test/product.h +++ b/test/product.h @@ -67,7 +67,7 @@ template void product(const MatrixType& m) RowVectorType v1 = RowVectorType::Random(rows), v2 = RowVectorType::Random(rows), vzero = RowVectorType::Zero(rows); - ColVectorType vc2 = ColVectorType::Random(cols), vcres; + ColVectorType vc2 = ColVectorType::Random(cols), vcres(cols); OtherMajorMatrixType tm1 = m1; Scalar s1 = ei_random(); -- cgit v1.2.3