Re: [CHANGESET]: First attempt at a single precision type.

[John W. Eaton]

On 28-Apr-2008, David Bateman wrote:

| The code duplication is not a big issue to me.. Templates are suppose to
| make it easy to add new code based on existing templates.. With existing
| untemplated code its easier just to copy the code and do regexp
| replacement on
| 
| double -> float
| Complex -> FloatComplex
| Matrix -> FloatMatrix
| NDArray -> FloatNDArray
| RowVector -> FloatRowVector
| ColumnVector -> FloatColumnVector
| ComplexFloat -> Complex
| ieee_lo - > ieee_lo_float
| octave_Inf -> octave_Float_Inf
| octave_NaN -> octave_Float_NaN
| octave_NA -> octave_Float_NA
| 
| and 99% of the conversion on the liboctave code is already done..

I agree that this is easy to do initially, but I think it could lead
to a maintenance problem later.  I generally don't enjoy having to fix
a bug in multiple places and having multiple versions of essentially
the same function makes it much more likely that there will be some
divergence and the risk of subtley different bugs in each.  As you
say, it makes sense to push as much as possible to the MArray2 and
MArrayN classes.  I just think we should try to use templates for even
some of those functions where it might not be immediately obvious that
a template solution is possible (for example, due to the need to call
different BLAS or LAPACK functions).

| That being said, I'd suggest the way forward is to take the existing
| code, and migrate as appropriate some of the untemplated code into the
| MArray2 and MArrayN classes. Good candidates for this are is_symmetric
| and fill. Other code such as append, stack, imag, real, etc might be
| made a template function, but there would need to have untemplated
| versions to convert the MArray2<T> and MArrayN<T> return types to the
| derived class.

I would expect we could handle those with simple inline wrappers
defined in .h files.  For example, see the attached patch.  It would
not be difficult to extend this to mixed types (complex/float/double),
and to also handle the return type automatically by using a traits
class.  It will take a little more work to handle stacking matrices
and row/column vectors and diag matrices.

| Should changes would also introduce dependencies of the
| MArray2 and MArrayN classes on the mx_* functions and so this might
| cause issues with the user types for galois, fixed, symbolic, etc in
| octave-forge. So maybe just to functions that don't introduce such
| dependencies should be templated..

Which functions are you thinking of that might cause trouble?

jwe

diff --git a/liboctave/Array.cc b/liboctave/Array.cc
--- a/liboctave/Array.cc
+++ b/liboctave/Array.cc
@@ -1192,6 +1192,33 @@ Array<T>::insert (const Array<T>& a, con
       ("Array<T>::insert: invalid indexing operation");
 
   return *this;
+}
+
+template <class T>
+Array<T>
+Array<T>::stack (const Array<T>& a) const
+{
+  assert (ndims () == 2);
+
+  octave_idx_type nr = dim1 ();
+  octave_idx_type nc = dim2 ();
+
+  octave_idx_type a_nr = a.dim1 ();
+  octave_idx_type a_nc = a.dim2 ();
+
+  if (nc != a_nc)
+    {
+      (*current_liboctave_error_handler)
+       ("column dimension mismatch for stack");
+      return Array<T> ();
+    }
+
+  Array<T> retval (dim_vector (nr + a_nr, nc));
+
+  retval.insert (*this, 0, 0);
+  retval.insert (a, nr, 0);
+
+  return retval;
 }
 
 template <class T>
diff --git a/liboctave/Array.h b/liboctave/Array.h
--- a/liboctave/Array.h
+++ b/liboctave/Array.h
@@ -460,6 +460,8 @@ public:
 
   bool is_empty (void) const { return numel () == 0; }
 
+  Array<T> stack (const Array<T>& a) const;
+
   Array<T> transpose (void) const;
 
   const T *data (void) const { return rep->data; }
diff --git a/liboctave/Array2.h b/liboctave/Array2.h
--- a/liboctave/Array2.h
+++ b/liboctave/Array2.h
@@ -103,6 +103,8 @@ public:
       return *this;
     }
 
+  Array2<T> stack (const Array2<T>& a) const { return Array<T>::stack (a); }
+
   Array2<T> transpose (void) const
     {
       Array<T> tmp = Array<T>::transpose ();
diff --git a/liboctave/CMatrix.cc b/liboctave/CMatrix.cc
--- a/liboctave/CMatrix.cc
+++ b/liboctave/CMatrix.cc
@@ -809,25 +809,6 @@ ComplexMatrix::stack (const DiagMatrix& 
 }
 
 ComplexMatrix
-ComplexMatrix::stack (const ComplexMatrix& a) const
-{
-  octave_idx_type nr = rows ();
-  octave_idx_type nc = cols ();
-  if (nc != a.cols ())
-    {
-      (*current_liboctave_error_handler)
-       ("column dimension mismatch for stack");
-      return *this;
-    }
-
-  octave_idx_type nr_insert = nr;
-  ComplexMatrix retval (nr + a.rows (), nc);
-  retval.insert (*this, 0, 0);
-  retval.insert (a, nr_insert, 0);
-  return retval;
-}
-
-ComplexMatrix
 ComplexMatrix::stack (const ComplexRowVector& a) const
 {
   octave_idx_type nr = rows ();
diff --git a/liboctave/CMatrix.h b/liboctave/CMatrix.h
--- a/liboctave/CMatrix.h
+++ b/liboctave/CMatrix.h
@@ -117,7 +117,7 @@ public:
   ComplexMatrix stack (const ColumnVector& a) const;
   ComplexMatrix stack (const DiagMatrix& a) const;
 
-  ComplexMatrix stack (const ComplexMatrix& a) const;
+  ComplexMatrix stack (const ComplexMatrix& a) const { return 
MArray2<Complex>::stack (a); }
   ComplexMatrix stack (const ComplexRowVector& a) const;
   ComplexMatrix stack (const ComplexColumnVector& a) const;
   ComplexMatrix stack (const ComplexDiagMatrix& a) const;
diff --git a/liboctave/MArray2.h b/liboctave/MArray2.h
--- a/liboctave/MArray2.h
+++ b/liboctave/MArray2.h
@@ -79,6 +79,8 @@ public:
     return *this;
   }
 
+  MArray2<T> stack (const MArray2<T>& a) const { return Array2<T>::stack (a); }
+
   MArray2<T> transpose (void) const { return Array2<T>::transpose (); }
 
   MArray2<T> diag (octave_idx_type k) const
diff --git a/liboctave/dMatrix.cc b/liboctave/dMatrix.cc
--- a/liboctave/dMatrix.cc
+++ b/liboctave/dMatrix.cc
@@ -480,25 +480,6 @@ Matrix::append (const DiagMatrix& a) con
   Matrix retval (nr, nc + a.cols ());
   retval.insert (*this, 0, 0);
   retval.insert (a, 0, nc_insert);
-  return retval;
-}
-
-Matrix
-Matrix::stack (const Matrix& a) const
-{
-  octave_idx_type nr = rows ();
-  octave_idx_type nc = cols ();
-  if (nc != a.cols ())
-    {
-      (*current_liboctave_error_handler)
-       ("column dimension mismatch for stack");
-      return Matrix ();
-    }
-
-  octave_idx_type nr_insert = nr;
-  Matrix retval (nr + a.rows (), nc);
-  retval.insert (*this, 0, 0);
-  retval.insert (a, nr_insert, 0);
   return retval;
 }
 
diff --git a/liboctave/dMatrix.h b/liboctave/dMatrix.h
--- a/liboctave/dMatrix.h
+++ b/liboctave/dMatrix.h
@@ -89,7 +89,7 @@ public:
   Matrix append (const ColumnVector& a) const;
   Matrix append (const DiagMatrix& a) const;
 
-  Matrix stack (const Matrix& a) const;
+  Matrix stack (const Matrix& a) const { return MArray2<double>::stack (a); }
   Matrix stack (const RowVector& a) const;
   Matrix stack (const ColumnVector& a) const;
   Matrix stack (const DiagMatrix& a) const;