rework Identity API: no longer restricted to square matrices

14 files changed, 67 insertions, 41 deletions

diff --git a/Eigen/src/Core/Identity.h b/Eigen/src/Core/Identity.h
index 93b3c7cae..41a383156 100644
--- a/Eigen/src/Core/Identity.h
+++ b/Eigen/src/Core/Identity.h
@@ -30,7 +30,7 @@
   *
   * \brief Expression of the identity matrix of some size.
   *
-  * \sa MatrixBase::identity(int)
+  * \sa MatrixBase::identity(), MatrixBase::identity(int,int), MatrixBase::setIdentity()
   */
 template<typename MatrixType> class Identity : NoOperatorEquals,
   public MatrixBase<typename MatrixType::Scalar, Identity<MatrixType> >
@@ -39,9 +39,12 @@ template<typename MatrixType> class Identity : NoOperatorEquals,
     typedef typename MatrixType::Scalar Scalar;
     friend class MatrixBase<Scalar, Identity<MatrixType> >;
     
-    Identity(int rows) : m_rows(rows)
+    Identity(int rows, int cols) : m_rows(rows), m_cols(cols)
     {
-      assert(rows > 0 && RowsAtCompileTime == ColsAtCompileTime);
+      assert(rows > 0
+          && (RowsAtCompileTime == Dynamic || RowsAtCompileTime == rows)
+          && cols > 0
+          && (ColsAtCompileTime == Dynamic || ColsAtCompileTime == cols));
     }
     
   private:
@@ -52,7 +55,7 @@ template<typename MatrixType> class Identity : NoOperatorEquals,
     
     const Identity& _ref() const { return *this; }
     int _rows() const { return m_rows; }
-    int _cols() const { return m_rows; }
+    int _cols() const { return m_cols; }
     
     Scalar _coeff(int row, int col) const
     {
@@ -60,65 +63,84 @@ template<typename MatrixType> class Identity : NoOperatorEquals,
     }
     
   protected:
-    const int m_rows;
+    const int m_rows, m_cols;
 };
 
-/** \returns an expression of the identity matrix of given type and size.
+/** \returns an expression of the identity matrix (not necessarily square).
   *
-  * \param rows The number of rows of the identity matrix to return. If *this has
-  *             fixed size, that size is used as the default argument for \a rows
-  *             and is then the only allowed value. If *this has dynamic size,
-  *             you can use any positive value for \a rows.
+  * The parameters \a rows and \a cols are the number of rows and of columns of
+  * the returned matrix. Must be compatible with this MatrixBase type.
   *
-  * \note An identity matrix is a square matrix, so it is required that the type of *this
-  *       allows being a square matrix.
+  * This variant is meant to be used for dynamic-size matrix types. For fixed-size types,
+  * it is redundant to pass \a rows and \a cols as arguments, so identity() should be used
+  * instead.
   *
-  * Example: \include MatrixBase_identity_int.cpp
-  * Output: \verbinclude MatrixBase_identity_int.out
+  * Example: \include MatrixBase_identity_int_int.cpp
+  * Output: \verbinclude MatrixBase_identity_int_int.out
   *
-  * \sa class Identity, isIdentity()
+  * \sa identity(), setIdentity(), isIdentity()
   */
 template<typename Scalar, typename Derived>
-const Identity<Derived> MatrixBase<Scalar, Derived>::identity(int rows)
+const Identity<Derived> MatrixBase<Scalar, Derived>::identity(int rows, int cols)
 {
-  return Identity<Derived>(rows);
+  return Identity<Derived>(rows, cols);
 }
 
-/** \returns true if *this is approximately equal to the identity matrix,
+/** \returns an expression of the identity matrix (not necessarily square).
+  *
+  * This variant is only for fixed-size MatrixBase types. For dynamic-size types, you
+  * need to use the variant taking size arguments.
+  *
+  * Example: \include MatrixBase_identity.cpp
+  * Output: \verbinclude MatrixBase_identity.out
+  *
+  * \sa identity(int,int), setIdentity(), isIdentity()
+  */
+template<typename Scalar, typename Derived>
+const Identity<Derived> MatrixBase<Scalar, Derived>::identity()
+{
+  return Identity<Derived>(Traits::RowsAtCompileTime, Traits::ColsAtCompileTime);
+}
+
+/** \returns true if *this is approximately equal to the identity matrix
+  *          (not necessarily square),
   *          within the precision given by \a prec.
   *
   * Example: \include MatrixBase_isIdentity.cpp
   * Output: \verbinclude MatrixBase_isIdentity.out
   *
-  * \sa class Identity, identity(int)
+  * \sa class Identity, identity(), identity(int,int), setIdentity()
   */
 template<typename Scalar, typename Derived>
 bool MatrixBase<Scalar, Derived>::isIdentity
 (typename NumTraits<Scalar>::Real prec) const
 {
-  if(cols() != rows()) return false;
   for(int j = 0; j < cols(); j++)
   {
-    if(!Eigen::isApprox(coeff(j, j), static_cast<Scalar>(1), prec))
-      return false;
-    for(int i = 0; i < j; i++)
-      if(!Eigen::isMuchSmallerThan(coeff(i, j), static_cast<Scalar>(1), prec))
-        return false;
+    for(int i = 0; i < rows(); i++)
+    {
+      if(i == j)
+        if(!Eigen::isApprox(coeff(i, j), static_cast<Scalar>(1), prec))
+          return false;
+      else
+        if(!Eigen::isMuchSmallerThan(coeff(i, j), static_cast<Scalar>(1), prec))
+          return false;
+    }
   }
   return true;
 }
 
-/** Writes the identity expression into *this.
+/** Writes the identity expression (not necessarily square) into *this.
   *
   * Example: \include MatrixBase_setIdentity.cpp
   * Output: \verbinclude MatrixBase_setIdentity.out
   *
-  * \sa class Identity, identity()
+  * \sa class Identity, identity(), identity(int,int), isIdentity()
   */
 template<typename Scalar, typename Derived>
 Derived& MatrixBase<Scalar, Derived>::setIdentity()
 {
-  return *this = Identity<Derived>(rows());
+  return *this = Identity<Derived>(rows(), cols());
 }
 
 
diff --git a/Eigen/src/Core/MatrixBase.h b/Eigen/src/Core/MatrixBase.h
index 3a62b8e34..2fd2bb86c 100644
--- a/Eigen/src/Core/MatrixBase.h
+++ b/Eigen/src/Core/MatrixBase.h
@@ -182,7 +182,8 @@ template<typename Scalar, typename Derived> class MatrixBase
     static const Ones<Derived> ones(int rows, int cols);
     static const Ones<Derived> ones(int size);
     static const Ones<Derived> ones();
-    static const Identity<Derived> identity(int rows = Derived::RowsAtCompileTime);
+    static const Identity<Derived> identity();
+    static const Identity<Derived> identity(int rows, int cols);
     
     bool isZero(RealScalar prec = precision<Scalar>()) const;
     bool isOnes(RealScalar prec = precision<Scalar>()) const;
diff --git a/Eigen/src/Core/Ones.h b/Eigen/src/Core/Ones.h
index bb52889c9..1d3ee6af0 100644
--- a/Eigen/src/Core/Ones.h
+++ b/Eigen/src/Core/Ones.h
@@ -30,7 +30,8 @@
   *
   * \brief Expression of a matrix where all coefficients equal one.
   *
-  * \sa MatrixBase::ones(), MatrixBase::ones(int), MatrixBase::ones(int,int)
+  * \sa MatrixBase::ones(), MatrixBase::ones(int), MatrixBase::ones(int,int),
+  *     MatrixBase::setOnes(), MatrixBase::isOnes()
   */
 template<typename MatrixType> class Ones : NoOperatorEquals,
   public MatrixBase<typename MatrixType::Scalar, Ones<MatrixType> >
diff --git a/Eigen/src/Core/Random.h b/Eigen/src/Core/Random.h
index f07fa792a..6a8a9e726 100644
--- a/Eigen/src/Core/Random.h
+++ b/Eigen/src/Core/Random.h
@@ -30,7 +30,8 @@
   *
   * \brief Expression of a random matrix or vector.
   *
-  * \sa MatrixBase::random(), MatrixBase::random(int), MatrixBase::random(int,int)
+  * \sa MatrixBase::random(), MatrixBase::random(int), MatrixBase::random(int,int),
+  *     MatrixBase::setRandom()
   */
 template<typename MatrixType> class Random : NoOperatorEquals,
   public MatrixBase<typename MatrixType::Scalar, Random<MatrixType> >
diff --git a/Eigen/src/Core/Zero.h b/Eigen/src/Core/Zero.h
index f90bb2849..40ee926ed 100644
--- a/Eigen/src/Core/Zero.h
+++ b/Eigen/src/Core/Zero.h
@@ -30,7 +30,8 @@
   *
   * \brief Expression of a zero matrix or vector.
   *
-  * \sa MatrixBase::zero(), MatrixBase::zero(int), MatrixBase::zero(int,int)
+  * \sa MatrixBase::zero(), MatrixBase::zero(int), MatrixBase::zero(int,int),
+  *     MatrixBase::setZero(), MatrixBase::isZero()
   */
 template<typename MatrixType> class Zero : NoOperatorEquals,
   public MatrixBase<typename MatrixType::Scalar, Zero<MatrixType> >
diff --git a/doc/snippets/MatrixBase_identity.cpp b/doc/snippets/MatrixBase_identity.cpp
new file mode 100644
index 000000000..cac2a7205
--- /dev/null
+++ b/doc/snippets/MatrixBase_identity.cpp
@@ -0,0 +1 @@
+cout << Matrix<double, 3, 4>::identity() << endl;
diff --git a/doc/snippets/MatrixBase_identity_int.cpp b/doc/snippets/MatrixBase_identity_int.cpp
deleted file mode 100644
index 8cf39d93d..000000000
--- a/doc/snippets/MatrixBase_identity_int.cpp
+++ /dev/null
@@ -1,2 +0,0 @@
-cout << Matrix2d::identity() << endl;
-cout << MatrixXd::identity(3) << endl;
diff --git a/doc/snippets/MatrixBase_identity_int_int.cpp b/doc/snippets/MatrixBase_identity_int_int.cpp
new file mode 100644
index 000000000..5b2adfc20
--- /dev/null
+++ b/doc/snippets/MatrixBase_identity_int_int.cpp
@@ -0,0 +1 @@
+cout << MatrixXd::identity(4, 3) << endl;
diff --git a/test/adjoint.cpp b/test/adjoint.cpp
index 4b6b692e4..f6cd53631 100644
--- a/test/adjoint.cpp
+++ b/test/adjoint.cpp
@@ -43,7 +43,7 @@ template<typename MatrixType> void adjoint(const MatrixType& m)
              m3(rows, cols),
              mzero = MatrixType::zero(rows, cols),
              identity = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
-                              ::identity(rows),
+                              ::identity(rows, rows),
              square = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
                               ::random(rows, rows);
   VectorType v1 = VectorType::random(rows),
diff --git a/test/basicstuff.cpp b/test/basicstuff.cpp
index d5732dde7..f8a651a73 100644
--- a/test/basicstuff.cpp
+++ b/test/basicstuff.cpp
@@ -42,7 +42,7 @@ template<typename MatrixType> void basicStuff(const MatrixType& m)
              m3(rows, cols),
              mzero = MatrixType::zero(rows, cols),
              identity = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
-                              ::identity(rows),
+                              ::identity(rows, rows),
              square = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
                               ::random(rows, rows);
   VectorType v1 = VectorType::random(rows),
diff --git a/test/linearstructure.cpp b/test/linearstructure.cpp
index c90488dd0..682315c7a 100644
--- a/test/linearstructure.cpp
+++ b/test/linearstructure.cpp
@@ -46,7 +46,7 @@ template<typename MatrixType> void linearStructure(const MatrixType& m)
              m3(rows, cols),
              mzero = MatrixType::zero(rows, cols),
              identity = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
-                              ::identity(rows),
+                              ::identity(rows, rows),
              square = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
                               ::random(rows, rows);
   VectorType v1 = VectorType::random(rows),
diff --git a/test/miscmatrices.cpp b/test/miscmatrices.cpp
index 57d6f94eb..42f808b1a 100644
--- a/test/miscmatrices.cpp
+++ b/test/miscmatrices.cpp
@@ -51,7 +51,7 @@ template<typename MatrixType> void miscMatrices(const MatrixType& m)
   else VERIFY_IS_MUCH_SMALLER_THAN(square(r,r2), static_cast<Scalar>(1));
   square = MatrixType::zero(rows, rows);
   square.diagonal() = VectorType::ones(rows);
-  VERIFY_IS_APPROX(square, MatrixType::identity(rows));
+  VERIFY_IS_APPROX(square, MatrixType::identity(rows, rows));
 }
 
 void EigenTest::testMiscMatrices()
diff --git a/test/product.cpp b/test/product.cpp
index 449c0f3f7..9bba45c48 100644
--- a/test/product.cpp
+++ b/test/product.cpp
@@ -46,7 +46,7 @@ template<typename MatrixType> void product(const MatrixType& m)
              m3(rows, cols),
              mzero = MatrixType::zero(rows, cols),
              identity = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
-                              ::identity(rows),
+                              ::identity(rows, rows),
              square = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
                               ::random(rows, rows);
   VectorType v1 = VectorType::random(rows),
@@ -83,7 +83,7 @@ template<typename MatrixType> void product(const MatrixType& m)
   VERIFY_IS_APPROX(m1,                      identity*m1);
   VERIFY_IS_APPROX(v1,                      identity*v1);
   // again, test operator() to check const-qualification
-  VERIFY_IS_APPROX(MatrixType::identity(std::max(rows,cols))(r,c), static_cast<Scalar>(r==c));
+  VERIFY_IS_APPROX(MatrixType::identity(rows, cols)(r,c), static_cast<Scalar>(r==c));
 }
 
 void EigenTest::testProduct()
diff --git a/test/submatrices.cpp b/test/submatrices.cpp
index 9981d9f83..ee0e49642 100644
--- a/test/submatrices.cpp
+++ b/test/submatrices.cpp
@@ -44,7 +44,7 @@ template<typename MatrixType> void submatrices(const MatrixType& m)
              m3(rows, cols),
              mzero = MatrixType::zero(rows, cols),
              identity = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
-                              ::identity(rows),
+                              ::identity(rows, rows),
              square = Matrix<Scalar, MatrixType::Traits::RowsAtCompileTime, MatrixType::Traits::RowsAtCompileTime>
                               ::random(rows, rows);
   VectorType v1 = VectorType::random(rows),