[Changeset]: octave_value(const ArrayN<octave_idx_type>&) constructor

[David Bateman]

The attached changeset adds a constructor for ArrayN<octave_idx_type>
that seems to me might be used whether indexes are created. It then uses
it in Flookup.

D.

--
David Bateman                                David.Bateman@...
Motorola Labs - Paris                        [phone] (Ph)
Parc Les Algorithmes, Commune de St Aubin    [phone] (Mob)
91193 Gif-Sur-Yvette FRANCE                  [phone] (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary


# HG changeset patch
# User David Bateman <dbateman@...>
# Date 1214394450 -7200
# Node ID 54f8e3ad246db8ab9daf0463dec9687278712fae
# Parent  a3e7e24a6359c9c962390c6d364952d685eb1664
new octave_value constructor for ArrayN<octave_idx_type>

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,11 @@ 2008-06-25  David Bateman  <dbateman@fre
 2008-06-25  David Bateman  <dbateman@...>
 
+ * ov.cc (octave_value const ArrayN<octave_idx_type)): New
+ constructor
+ * ov.h (octave_value const ArrayN<octave_idx_type)): Declare it.
+ * DLD-FUNCTIONS/lookup.cc (assign): Delete.
+ (Flookup): Use new octave_value constructors rather than assign.
+
  * pr-output.cc (Frats): Print usage if nargin == 0.
 
 2008-06-10  John W. Eaton  <jwe@...>
diff --git a/src/DLD-FUNCTIONS/lookup.cc b/src/DLD-FUNCTIONS/lookup.cc
--- a/src/DLD-FUNCTIONS/lookup.cc
+++ b/src/DLD-FUNCTIONS/lookup.cc
@@ -47,25 +47,6 @@ contains_char (const std::string& str, c
 {
   return (str.find (c) != std::string::npos
   || str.find (std::toupper (c)) != std::string::npos);
-}
-
-// FIXME -- remove these one once octave_value supports octave_idx_type.
-static octave_value&
-assign (octave_value& ov, octave_idx_type idx)
-{
-  double tmp = idx;
-  ov = tmp;
-  return ov;
-}
-
-static octave_value&
-assign (octave_value& ov, const ArrayN<octave_idx_type>& ida)
-{
-  NDArray tmp (ida.dims ());
-  for (int i = 0; i < ida.numel (); i++)
-    tmp(i) = ida(i);
-  ov = tmp;
-  return ov;
 }
 
 // normal ascending comparator
@@ -239,8 +220,7 @@ For string lookup, 'i' indicates case-in
  std::less<double> ());
  }
 
-      //retval(0) = idx;
-      assign (retval(0), idx);
+      retval(0) = idx;
     }
   else if (str_case)
     {
@@ -288,8 +268,7 @@ For string lookup, 'i' indicates case-in
                   idx(i) = bin_lookup (table.data (), table.length (), y(i),
                                        std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = idx;
             }
           else
             {
@@ -298,8 +277,7 @@ For string lookup, 'i' indicates case-in
               idx = bin_lookup (table.data (), table.length (), argy,
                                 std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = idx;
             }
         }
       else
diff --git a/src/ov.cc b/src/ov.cc
--- a/src/ov.cc
+++ b/src/ov.cc
@@ -564,6 +564,15 @@ octave_value::octave_value (const ArrayN
 octave_value::octave_value (const ArrayN<float>& a)
   : rep (new octave_float_matrix (a))
 {
+  maybe_mutate ();
+}
+
+octave_value::octave_value (const ArrayN<octave_idx_type>& a)
+{
+  NDArray tmp (a.dims ());
+  for (octave_idx_type i = 0; i < a.numel (); i++)
+    tmp(i) = a(i);
+  rep = new octave_matrix (tmp);
   maybe_mutate ();
 }
 
diff --git a/src/ov.h b/src/ov.h
--- a/src/ov.h
+++ b/src/ov.h
@@ -189,6 +189,7 @@ public:
   octave_value (const FloatNDArray& nda);
   octave_value (const ArrayN<double>& m);
   octave_value (const ArrayN<float>& m);
+  octave_value (const ArrayN<octave_idx_type>& m);
   octave_value (const DiagMatrix& d);
   octave_value (const FloatDiagMatrix& d);
   octave_value (const RowVector& v);

[John W. Eaton]

On 25-Jun-2008, David Bateman wrote:

| The attached changeset adds a constructor for ArrayN<octave_idx_type>
| that seems to me might be used whether indexes are created. It then uses
| it in Flookup.

| diff --git a/src/ov.cc b/src/ov.cc
| --- a/src/ov.cc
| +++ b/src/ov.cc
| @@ -564,6 +564,15 @@ octave_value::octave_value (const ArrayN
|  octave_value::octave_value (const ArrayN<float>& a)
|    : rep (new octave_float_matrix (a))
|  {
| +  maybe_mutate ();
| +}
| +
| +octave_value::octave_value (const ArrayN<octave_idx_type>& a)
| +{
| +  NDArray tmp (a.dims ());
| +  for (octave_idx_type i = 0; i < a.numel (); i++)
| +    tmp(i) = a(i);
| +  rep = new octave_matrix (tmp);
|    maybe_mutate ();
|  }

It might be better to use

  double *ptmp = tmp.fortran_vec ();
  for (octave_idx_type i = 0; i < a.numel (); i++)
    ptmp[i] = a(i);

to avoid repeatedly checking the reference count.  This is a change
that we should consider in many other places as well, if someone is
looking for a small project...

Also, should we do anything about octave_idx_type values that can't be
represented exactly in an NDArray element?

jwe

[David Bateman]

John W. Eaton wrote:
> It might be better to use
>
>   double *ptmp = tmp.fortran_vec ();
>   for (octave_idx_type i = 0; i < a.numel (); i++)
>     ptmp[i] = a(i);
>  
That makes sense. Do you want to do it, or do you want a new patch.
> to avoid repeatedly checking the reference count.  This is a change
> that we should consider in many other places as well, if someone is
> looking for a small project...
>  
I didn't just do it straight away for things like Fsort, Ffind etc as
you have to be careful with zero and one based indexing. Maybe it would
be better if the constructor was in fact

octave_value (const ArrayN<octave_idx_type>& a, bool zero_base = false);

octave_value::octave_value (const ArrayN<octave_idx_type>& a, bool
zero_base)
{
  NDArray tmp (a.dims ());
  double *ptmp = tmp.fortran_vec ();
  if (zero_based)
    for (octave_idx_type i = 0; i < a.numel (); i++)
      ptmp[i] = a(i) + 1;
  else
    for (octave_idx_type i = 0; i < a.numel (); i++)
      ptmp[i] = a(i);
  rep = new octave_matrix (tmp);
  maybe_mutate ();
 }

to allow this to be more easily handled.

> Also, should we do anything about octave_idx_type values that can't be
> represented exactly in an NDArray element?
>  
2^52 is a very big number I think we can wait till we have to be careful
with that few a few decades at least :-)

D.

--
David Bateman                                David.Bateman@...
Motorola Labs - Paris                        [phone] (Ph)
Parc Les Algorithmes, Commune de St Aubin    [phone] (Mob)
91193 Gif-Sur-Yvette FRANCE                  [phone] (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

[Jaroslav Hajek-2]

On Wed, Jun 25, 2008 at 2:00 PM, John W. Eaton <jwe@...> wrote:

> On 25-Jun-2008, David Bateman wrote:
>
> | The attached changeset adds a constructor for ArrayN<octave_idx_type>
> | that seems to me might be used whether indexes are created. It then uses
> | it in Flookup.
>
> | diff --git a/src/ov.cc b/src/ov.cc
> | --- a/src/ov.cc
> | +++ b/src/ov.cc
> | @@ -564,6 +564,15 @@ octave_value::octave_value (const ArrayN
> |  octave_value::octave_value (const ArrayN<float>& a)
> |    : rep (new octave_float_matrix (a))
> |  {
> | +  maybe_mutate ();
> | +}
> | +
> | +octave_value::octave_value (const ArrayN<octave_idx_type>& a)
> | +{
> | +  NDArray tmp (a.dims ());
> | +  for (octave_idx_type i = 0; i < a.numel (); i++)
> | +    tmp(i) = a(i);
> | +  rep = new octave_matrix (tmp);
> |    maybe_mutate ();
> |  }
>
> It might be better to use
>
>  double *ptmp = tmp.fortran_vec ();
>  for (octave_idx_type i = 0; i < a.numel (); i++)
>    ptmp[i] = a(i);
>

Or use "tmp.xelem(i) = a(i);"

> to avoid repeatedly checking the reference count.  This is a change
> that we should consider in many other places as well, if someone is
> looking for a small project...
>
> Also, should we do anything about octave_idx_type values that can't be
> represented exactly in an NDArray element?
>

I think we have discussed that before, when lookup was comitted, and
the conclusion was that the compatibility problems outweigh the
benefits.

> jwe
>
>
>
>



--
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz

[David Bateman]

consider the attached changeset instead.

--
David Bateman                                David.Bateman@...
Motorola Labs - Paris                        [phone] (Ph)
Parc Les Algorithmes, Commune de St Aubin    [phone] (Mob)
91193 Gif-Sur-Yvette FRANCE                  [phone] (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary


# HG changeset patch
# User David Bateman <dbateman@...>
# Date 1214493236 -7200
# Node ID 8907376d97039307c7dee973ab5d4f6bb09048a2
# Parent  191d10e982d62fb9b218a00289b715cbe82d8e7a
new octave_value constructor for ArrayN<octave_idx_type>

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,22 @@ 2008-06-23  Jaroslav Hajek <highegg@gmai
[phone]  David Bateman  <dbateman@...>
+
+ * ov.cc (octave_value::octave_value const ArrayN<octave_idx_type>,
+ bool)): New constructor
+ * ov.h (octave_value const ArrayN<octave_idx_type>, bool)): Declare it.
+ * DLD-FUNCTIONS/lookup.cc (assign): Delete.
+ (Flookup): Use new octave_value constructors rather than assign.
+ * data.cc (Fsort): Use new octave_value constructors rather than
+ directly assigning.
+ * besselj.cc (int_array2_to_matrix, int_arrayn_to_array,
+ int_array2_to_float_matrix, int_arrayn_to_float_array): Delete.
+ (do-bessel): Use new octave_value constructors.
+ * max.cc (MINMAX_SPARSE_BODY, MINMAX_INT_BODY, MINMAX_SINGLE_BODY,
+ MINMAX_DOUBLE_BODY): Use new octave_value constructors.
+
[phone]  David Bateman  <dbateman@...>
+
+ * pr-output.cc (Frats): Print usage if nargin == 0.
+
 2008-06-23  Jaroslav Hajek <highegg@...>
 
  * genprops.awk (emit_source): Avoid gensub for portability.
@@ -70,10 +89,6 @@ 2008-06-11  John W. Eaton  <jwe@...
  * error.cc (verror): Restore newline in msg_string.  Stripping
  "error: " prefix when buffering error messages is no longer neeed.
 
-2008-06-25  David Bateman  <dbateman@...>
-
- * pr-output.cc (Frats): Print usage if nargin == 0.
-
 2008-06-10  John W. Eaton  <jwe@...>
 
  * mexproto.h (mxCreateLogicalScalar): Declar arg as mxLogical, not int.
diff --git a/src/DLD-FUNCTIONS/besselj.cc b/src/DLD-FUNCTIONS/besselj.cc
--- a/src/DLD-FUNCTIONS/besselj.cc
+++ b/src/DLD-FUNCTIONS/besselj.cc
@@ -79,80 +79,6 @@ enum bessel_type
     } \
   while (0)
 
-static inline Matrix
-int_array2_to_matrix (const Array2<octave_idx_type>& a)
-{
-  octave_idx_type nr = a.rows ();
-  octave_idx_type nc = a.cols ();
-
-  Matrix retval (nr, nc);
-
-  for (octave_idx_type j = 0; j < nc; j++)
-    for (octave_idx_type i = 0; i < nr; i++)
-      {
- OCTAVE_QUIT;
-
- retval(i,j) = static_cast<double> (a(i,j));
-      }
-
-  return retval;
-}
-
-static inline NDArray
-int_arrayN_to_array (const ArrayN<octave_idx_type>& a)
-{
-  dim_vector dv = a.dims ();
-  int nel = dv.numel ();
-
-  NDArray retval (dv);
-
-  for (int i = 0; i < nel; i++)
-    {
-      OCTAVE_QUIT;
-      
-      retval(i) = static_cast<double> (a(i));
-    }
-
-  return retval;
-}
-
-static inline FloatMatrix
-int_array2_to_float_matrix (const Array2<octave_idx_type>& a)
-{
-  octave_idx_type nr = a.rows ();
-  octave_idx_type nc = a.cols ();
-
-  FloatMatrix retval (nr, nc);
-
-  for (octave_idx_type j = 0; j < nc; j++)
-    for (octave_idx_type i = 0; i < nr; i++)
-      {
- OCTAVE_QUIT;
-
- retval(i,j) = static_cast<float> (a(i,j));
-      }
-
-  return retval;
-}
-
-static inline FloatNDArray
-int_arrayN_to_float_array (const ArrayN<octave_idx_type>& a)
-{
-  dim_vector dv = a.dims ();
-  int nel = dv.numel ();
-
-  FloatNDArray retval (dv);
-
-  for (int i = 0; i < nel; i++)
-    {
-      OCTAVE_QUIT;
-      
-      retval(i) = static_cast<float> (a(i));
-    }
-
-  return retval;
-}
-
 static void
 gripe_bessel_arg (const char *fn, const char *arg)
 {
@@ -213,7 +139,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_float_array (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
@@ -249,7 +175,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, ralpha, cx, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_array2_to_float_matrix (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
@@ -277,7 +203,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
       if (nargout > 1)
- retval(1) = int_arrayN_to_float_array (ierr);
+ retval(1) = ierr;
 
       retval(0) = result;
     }
@@ -296,7 +222,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
   
       if (nargout > 1)
- retval(1) = int_arrayN_to_float_array (ierr);
+ retval(1) = ierr;
 
       retval(0) = result;
     }
@@ -348,7 +274,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_array (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
@@ -384,7 +310,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, ralpha, cx, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_array2_to_matrix (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
@@ -412,7 +338,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
       if (nargout > 1)
- retval(1) = int_arrayN_to_array (ierr);
+ retval(1) = ierr;
 
       retval(0) = result;
     }
@@ -431,7 +357,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
   
       if (nargout > 1)
- retval(1) = int_arrayN_to_array (ierr);
+ retval(1) = ierr;
 
       retval(0) = result;
     }
@@ -666,7 +592,7 @@ return @code{NaN}.\n\
     result = airy (z, kind == 1, scale, ierr);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_float_array (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
@@ -688,7 +614,7 @@ return @code{NaN}.\n\
     result = airy (z, kind == 1, scale, ierr);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_array (ierr);
+    retval(1) = ierr;
 
   retval(0) = result;
  }
diff --git a/src/DLD-FUNCTIONS/lookup.cc b/src/DLD-FUNCTIONS/lookup.cc
--- a/src/DLD-FUNCTIONS/lookup.cc
+++ b/src/DLD-FUNCTIONS/lookup.cc
@@ -47,25 +47,6 @@ contains_char (const std::string& str, c
 {
   return (str.find (c) != std::string::npos
   || str.find (std::toupper (c)) != std::string::npos);
-}
-
-// FIXME -- remove these one once octave_value supports octave_idx_type.
-static octave_value&
-assign (octave_value& ov, octave_idx_type idx)
-{
-  double tmp = idx;
-  ov = tmp;
-  return ov;
-}
-
-static octave_value&
-assign (octave_value& ov, const ArrayN<octave_idx_type>& ida)
-{
-  NDArray tmp (ida.dims ());
-  for (int i = 0; i < ida.numel (); i++)
-    tmp(i) = ida(i);
-  ov = tmp;
-  return ov;
 }
 
 // normal ascending comparator
@@ -239,8 +220,7 @@ For string lookup, 'i' indicates case-in
  std::less<double> ());
  }
 
-      //retval(0) = idx;
-      assign (retval(0), idx);
+      retval(0) = idx;
     }
   else if (str_case)
     {
@@ -288,8 +268,7 @@ For string lookup, 'i' indicates case-in
                   idx(i) = bin_lookup (table.data (), table.length (), y(i),
                                        std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = idx;
             }
           else
             {
@@ -298,8 +277,7 @@ For string lookup, 'i' indicates case-in
               idx = bin_lookup (table.data (), table.length (), argy,
                                 std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = idx;
             }
         }
       else
diff --git a/src/DLD-FUNCTIONS/max.cc b/src/DLD-FUNCTIONS/max.cc
--- a/src/DLD-FUNCTIONS/max.cc
+++ b/src/DLD-FUNCTIONS/max.cc
@@ -100,21 +100,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = octave_value (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
@@ -276,23 +262,9 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  float nan_val = lo_ieee_nan_value (); \
- \
-  FloatNDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<float> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
-      else \
- retval(1) = FloatNDArray (); \
+ retval(1) = octave_value (index, true, true); \
+      else \
+ retval(1) = NDArray (); \
     } \
   else \
     { \
@@ -423,21 +395,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = octave_value (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
@@ -518,21 +476,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = octave_value (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
diff --git a/src/data.cc b/src/data.cc
--- a/src/data.cc
+++ b/src/data.cc
@@ -5250,16 +5250,7 @@ ordered lists.\n\
       Array<octave_idx_type> sidx;
 
       retval (0) = arg.sort (sidx, dim, smode);
-
-      octave_idx_type *ps = sidx.fortran_vec ();
-      NDArray midx (sidx.dims ());
-      double *pm = midx.fortran_vec ();
-
-      for (octave_idx_type i = 0; i < sidx.numel (); i++)
- pm [i] = static_cast<double>
-  (ps [i] + static_cast<octave_idx_type> (1));
-
-      retval (1) = midx;
+      retval (1) = octave_value (sidx, true);
     }
   else
     retval(0) = arg.sort (dim, smode);
diff --git a/src/ov.cc b/src/ov.cc
--- a/src/ov.cc
+++ b/src/ov.cc
@@ -563,6 +563,94 @@ octave_value::octave_value (const ArrayN
 octave_value::octave_value (const ArrayN<float>& a)
   : rep (new octave_float_matrix (a))
 {
+  maybe_mutate ();
+}
+
+octave_value::octave_value (const ArrayN<octave_idx_type>& a, bool zero_based,
+    bool negative_to_nan)
+{
+  NDArray tmp (a.dims ());
+  const octave_idx_type *pa = a.fortran_vec ();
+  double *ptmp = tmp.fortran_vec ();
+  if (negative_to_nan)
+    {
+      double nan_val = lo_ieee_nan_value ();
+
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double>
+      (pa[i] + static_cast<octave_idx_type> (1));
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double> (pa[i]);
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+    }
+  else
+    {
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double>
+    (pa[i] + static_cast<octave_idx_type> (1));
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double> (pa[i]);
+    }
+  rep = new octave_matrix (tmp);
+  maybe_mutate ();
+}
+
+octave_value::octave_value (const Array<octave_idx_type>& a, bool zero_based,
+    bool negative_to_nan)
+{
+  NDArray tmp (a.dims ());
+  const octave_idx_type *pa = a.fortran_vec ();
+  double *ptmp = tmp.fortran_vec ();
+  if (negative_to_nan)
+    {
+      double nan_val = lo_ieee_nan_value ();
+
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double>
+      (pa[i] + static_cast<octave_idx_type> (1));
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double> (pa[i]);
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+    }
+  else
+    {
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double>
+    (pa[i] + static_cast<octave_idx_type> (1));
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double> (pa[i]);
+    }
+  rep = new octave_matrix (tmp);
   maybe_mutate ();
 }
 
diff --git a/src/ov.h b/src/ov.h
--- a/src/ov.h
+++ b/src/ov.h
@@ -187,6 +187,10 @@ public:
   octave_value (const FloatNDArray& nda);
   octave_value (const ArrayN<double>& m);
   octave_value (const ArrayN<float>& m);
+  octave_value (const ArrayN<octave_idx_type>& m, bool zero_based = false,
+ bool negative_to_nan = false);
+  octave_value (const Array<octave_idx_type>& m, bool zero_based = false,
+ bool negative_to_nan = false);
   octave_value (const DiagMatrix& d);
   octave_value (const FloatDiagMatrix& d);
   octave_value (const RowVector& v);

[John W. Eaton]

On 26-Jun-2008, David Bateman wrote:

| consider the attached changeset instead.

It seems odd to me that the octave_value constructor that takes an
Array<octave_idx_type> value would convert it to NDArray internally
instead of storing it as an Array<octave_idx_type>.  The other
conversions also seem odd, but I guess we already have a few other
constructors that have similar options.  What about defining
Array<octave_idx_type> to NDArray conversion with the options and then
using the existing octave_value (const NDArray&) constructor instead?
It seems to me that this approach might cause less trouble later if we
decided we wanted an actual Array<octave_idx_type> value in the
octave_value hierarchy.

jwe

[David Bateman]

John W. Eaton wrote:

> On 26-Jun-2008, David Bateman wrote:
>
> | consider the attached changeset instead.
>
> It seems odd to me that the octave_value constructor that takes an
> Array<octave_idx_type> value would convert it to NDArray internally
> instead of storing it as an Array<octave_idx_type>.  The other
> conversions also seem odd, but I guess we already have a few other
> constructors that have similar options.  What about defining
> Array<octave_idx_type> to NDArray conversion with the options and then
> using the existing octave_value (const NDArray&) constructor instead?
> It seems to me that this approach might cause less trouble later if we
> decided we wanted an actual Array<octave_idx_type> value in the
> octave_value hierarchy.
>
> jwe
>  

Ok, then here is take 3.

D.


# HG changeset patch
# User David Bateman <dbateman@...>
# Date 1214833891 -7200
# Node ID 2d683d51aa719e46d28aa93d7c9138acd6a48004
# Parent  9c14d5226c0b89bd7dc3daf6ac38bf57d203f6ce
new NDArray constructor for ArrayN<octave_idx_type>

diff --git a/liboctave/ChangeLog b/liboctave/ChangeLog
--- a/liboctave/ChangeLog
+++ b/liboctave/ChangeLog
@@ -1,3 +1,12 @@ 2008-06-20  Jaroslav Hajek <highegg@gmai
[phone]  David Bateman  <dbateman@...>
+
+ * dNDArray.cc (NDArray::NDArray (const ArrayN<octave_idx_type>&,
+ bool, bool), NDArray::NDArray (const Array<octave_idx_type>&,
+ bool, bool)): New constructors.
+ * dNDArray.h (NDArray::NDArray (const ArrayN<octave_idx_type>&,
+ bool, bool), NDArray::NDArray (const Array<octave_idx_type>&,
+ bool, bool)): Declare them.
+
 2008-06-20  Jaroslav Hajek <highegg@...>
 
  * MatrixType.h: Add missing include statement.
diff --git a/liboctave/dNDArray.cc b/liboctave/dNDArray.cc
--- a/liboctave/dNDArray.cc
+++ b/liboctave/dNDArray.cc
@@ -40,6 +40,90 @@ along with Octave; see the file COPYING.
 
 #if defined (HAVE_FFTW3)
 #include "oct-fftw.h"
+
+NDArray::NDArray (const ArrayN<octave_idx_type>& a, bool zero_based,
+  bool negative_to_nan)
+{
+  const octave_idx_type *pa = a.fortran_vec ();
+  resize (a.dims ());
+  double *ptmp = fortran_vec ();
+  if (negative_to_nan)
+    {
+      double nan_val = lo_ieee_nan_value ();
+
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double>
+      (pa[i] + static_cast<octave_idx_type> (1));
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double> (pa[i]);
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+    }
+  else
+    {
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double>
+    (pa[i] + static_cast<octave_idx_type> (1));
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double> (pa[i]);
+    }
+}
+
+NDArray::NDArray (const Array<octave_idx_type>& a, bool zero_based,
+  bool negative_to_nan)
+{
+  const octave_idx_type *pa = a.fortran_vec ();
+  resize (a.dims ());
+  double *ptmp = fortran_vec ();
+  if (negative_to_nan)
+    {
+      double nan_val = lo_ieee_nan_value ();
+
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double>
+      (pa[i] + static_cast<octave_idx_type> (1));
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  {
+    double val = static_cast<double> (pa[i]);
+    if (val <= 0)
+      ptmp[i] = nan_val;
+    else
+      ptmp[i] = val;
+  }
+    }
+  else
+    {
+      if (zero_based)
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double>
+    (pa[i] + static_cast<octave_idx_type> (1));
+      else
+ for (octave_idx_type i = 0; i < a.numel (); i++)
+  ptmp[i] = static_cast<double> (pa[i]);
+    }
+}
 
 ComplexNDArray
 NDArray::fourier (int dim) const
diff --git a/liboctave/dNDArray.h b/liboctave/dNDArray.h
--- a/liboctave/dNDArray.h
+++ b/liboctave/dNDArray.h
@@ -46,6 +46,12 @@ public:
   NDArray (const NDArray& a) : MArrayN<double> (a) { }
 
   NDArray (const Matrix& a) : MArrayN<double> (a) { }
+
+  NDArray (const ArrayN<octave_idx_type>& a, bool zero_based = false,
+   bool negative_to_nan = false);
+
+  NDArray (const Array<octave_idx_type>& a, bool zero_based = false,
+   bool negative_to_nan = false);
 
   template <class U>
   NDArray (const MArrayN<U>& a) : MArrayN<double> (a) { }
diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,17 @@ 2008-06-25  David Bateman  <dbateman@fre
[phone]  David Bateman  <dbateman@...>
+
+ * DLD-FUNCTIONS/lookup.cc (assign): Delete.
+ (Flookup): Use new NDArray constructors rather than assign.
+ * data.cc (Fsort): Use new NDArray constructors rather than
+ directly assigning.
+ * besselj.cc (int_array2_to_matrix, int_arrayn_to_array,
+ int_array2_to_float_matrix, int_arrayn_to_float_array): Delete.
+ (do-bessel): Use new NDArray constructors.
+ * max.cc (MINMAX_SPARSE_BODY, MINMAX_INT_BODY, MINMAX_SINGLE_BODY,
+ MINMAX_DOUBLE_BODY): Use new NDArray constructors.
+ *  ov-bool.h (array_value, float_array_value): explict cast
+ boolean scalar to double before the assignment to avoid ambiguity.
+
 2008-06-25  David Bateman  <dbateman@...>
 
  * pr-output.cc (Frats): Print usage if nargin == 0.
diff --git a/src/DLD-FUNCTIONS/besselj.cc b/src/DLD-FUNCTIONS/besselj.cc
--- a/src/DLD-FUNCTIONS/besselj.cc
+++ b/src/DLD-FUNCTIONS/besselj.cc
@@ -79,80 +79,6 @@ enum bessel_type
     } \
   while (0)
 
-static inline Matrix
-int_array2_to_matrix (const Array2<octave_idx_type>& a)
-{
-  octave_idx_type nr = a.rows ();
-  octave_idx_type nc = a.cols ();
-
-  Matrix retval (nr, nc);
-
-  for (octave_idx_type j = 0; j < nc; j++)
-    for (octave_idx_type i = 0; i < nr; i++)
-      {
- OCTAVE_QUIT;
-
- retval(i,j) = static_cast<double> (a(i,j));
-      }
-
-  return retval;
-}
-
-static inline NDArray
-int_arrayN_to_array (const ArrayN<octave_idx_type>& a)
-{
-  dim_vector dv = a.dims ();
-  int nel = dv.numel ();
-
-  NDArray retval (dv);
-
-  for (int i = 0; i < nel; i++)
-    {
-      OCTAVE_QUIT;
-      
-      retval(i) = static_cast<double> (a(i));
-    }
-
-  return retval;
-}
-
-static inline FloatMatrix
-int_array2_to_float_matrix (const Array2<octave_idx_type>& a)
-{
-  octave_idx_type nr = a.rows ();
-  octave_idx_type nc = a.cols ();
-
-  FloatMatrix retval (nr, nc);
-
-  for (octave_idx_type j = 0; j < nc; j++)
-    for (octave_idx_type i = 0; i < nr; i++)
-      {
- OCTAVE_QUIT;
-
- retval(i,j) = static_cast<float> (a(i,j));
-      }
-
-  return retval;
-}
-
-static inline FloatNDArray
-int_arrayN_to_float_array (const ArrayN<octave_idx_type>& a)
-{
-  dim_vector dv = a.dims ();
-  int nel = dv.numel ();
-
-  FloatNDArray retval (dv);
-
-  for (int i = 0; i < nel; i++)
-    {
-      OCTAVE_QUIT;
-      
-      retval(i) = static_cast<float> (a(i));
-    }
-
-  return retval;
-}
-
 static void
 gripe_bessel_arg (const char *fn, const char *arg)
 {
@@ -213,7 +139,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_float_array (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
@@ -249,7 +175,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, ralpha, cx, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_array2_to_float_matrix (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
@@ -277,7 +203,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
       if (nargout > 1)
- retval(1) = int_arrayN_to_float_array (ierr);
+ retval(1) = NDArray (ierr);
 
       retval(0) = result;
     }
@@ -296,7 +222,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
   
       if (nargout > 1)
- retval(1) = int_arrayN_to_float_array (ierr);
+ retval(1) = NDArray (ierr);
 
       retval(0) = result;
     }
@@ -348,7 +274,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_array (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
@@ -384,7 +310,7 @@ do_bessel (enum bessel_type type, const
   DO_BESSEL (type, ralpha, cx, scaled, ierr, result);
 
   if (nargout > 1)
-    retval(1) = int_array2_to_matrix (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
@@ -412,7 +338,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
 
       if (nargout > 1)
- retval(1) = int_arrayN_to_array (ierr);
+ retval(1) = NDArray (ierr);
 
       retval(0) = result;
     }
@@ -431,7 +357,7 @@ do_bessel (enum bessel_type type, const
       DO_BESSEL (type, alpha, x, scaled, ierr, result);
   
       if (nargout > 1)
- retval(1) = int_arrayN_to_array (ierr);
+ retval(1) = NDArray (ierr);
 
       retval(0) = result;
     }
@@ -666,7 +592,7 @@ return @code{NaN}.\n\
     result = airy (z, kind == 1, scale, ierr);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_float_array (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
@@ -688,7 +614,7 @@ return @code{NaN}.\n\
     result = airy (z, kind == 1, scale, ierr);
 
   if (nargout > 1)
-    retval(1) = int_arrayN_to_array (ierr);
+    retval(1) = NDArray (ierr);
 
   retval(0) = result;
  }
diff --git a/src/DLD-FUNCTIONS/lookup.cc b/src/DLD-FUNCTIONS/lookup.cc
--- a/src/DLD-FUNCTIONS/lookup.cc
+++ b/src/DLD-FUNCTIONS/lookup.cc
@@ -47,25 +47,6 @@ contains_char (const std::string& str, c
 {
   return (str.find (c) != std::string::npos
   || str.find (std::toupper (c)) != std::string::npos);
-}
-
-// FIXME -- remove these one once octave_value supports octave_idx_type.
-static octave_value&
-assign (octave_value& ov, octave_idx_type idx)
-{
-  double tmp = idx;
-  ov = tmp;
-  return ov;
-}
-
-static octave_value&
-assign (octave_value& ov, const ArrayN<octave_idx_type>& ida)
-{
-  NDArray tmp (ida.dims ());
-  for (int i = 0; i < ida.numel (); i++)
-    tmp(i) = ida(i);
-  ov = tmp;
-  return ov;
 }
 
 // normal ascending comparator
@@ -239,8 +220,7 @@ For string lookup, 'i' indicates case-in
  std::less<double> ());
  }
 
-      //retval(0) = idx;
-      assign (retval(0), idx);
+      retval(0) = NDArray (idx);
     }
   else if (str_case)
     {
@@ -288,8 +268,7 @@ For string lookup, 'i' indicates case-in
                   idx(i) = bin_lookup (table.data (), table.length (), y(i),
                                        std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = NDArray (idx);
             }
           else
             {
@@ -298,8 +277,7 @@ For string lookup, 'i' indicates case-in
               idx = bin_lookup (table.data (), table.length (), argy,
                                 std::ptr_fun (ov_str_comp));
 
-              //retval(0) = idx;
-              assign (retval(0), idx);
+              retval(0) = static_cast<double> (idx);
             }
         }
       else
diff --git a/src/DLD-FUNCTIONS/max.cc b/src/DLD-FUNCTIONS/max.cc
--- a/src/DLD-FUNCTIONS/max.cc
+++ b/src/DLD-FUNCTIONS/max.cc
@@ -100,21 +100,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = NDArray (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
@@ -276,23 +262,9 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  float nan_val = lo_ieee_nan_value (); \
- \
-  FloatNDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<float> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
-      else \
- retval(1) = FloatNDArray (); \
+ retval(1) = NDArray (index, true, true); \
+      else \
+ retval(1) = NDArray (); \
     } \
   else \
     { \
@@ -423,21 +395,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = NDArray (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
@@ -518,21 +476,7 @@ along with Octave; see the file COPYING.
       octave_idx_type len = index.numel (); \
  \
       if (len > 0) \
- { \
-  double nan_val = lo_ieee_nan_value (); \
- \
-  NDArray idx (index.dims ()); \
- \
-  for (octave_idx_type i = 0; i < len; i++) \
-    { \
-      OCTAVE_QUIT; \
-      octave_idx_type tmp = index.elem (i) + 1; \
-      idx.elem (i) = (tmp <= 0) \
- ? nan_val : static_cast<double> (tmp); \
-    } \
- \
-  retval(1) = idx; \
- } \
+ retval(1) = NDArray (index, true, true); \
       else \
  retval(1) = NDArray (); \
     } \
diff --git a/src/data.cc b/src/data.cc
--- a/src/data.cc
+++ b/src/data.cc
@@ -5250,16 +5250,7 @@ ordered lists.\n\
       Array<octave_idx_type> sidx;
 
       retval (0) = arg.sort (sidx, dim, smode);
-
-      octave_idx_type *ps = sidx.fortran_vec ();
-      NDArray midx (sidx.dims ());
-      double *pm = midx.fortran_vec ();
-
-      for (octave_idx_type i = 0; i < sidx.numel (); i++)
- pm [i] = static_cast<double>
-  (ps [i] + static_cast<octave_idx_type> (1));
-
-      retval (1) = midx;
+      retval (1) = NDArray (sidx, true);
     }
   else
     retval(0) = arg.sort (dim, smode);
diff --git a/src/ov-bool.h b/src/ov-bool.h
--- a/src/ov-bool.h
+++ b/src/ov-bool.h
@@ -134,10 +134,10 @@ public:
     { return FloatMatrix (1, 1, scalar); }
 
   NDArray array_value (bool = false) const
-    { return NDArray (dim_vector (1, 1), scalar); }
+    { return NDArray (dim_vector (1, 1), static_cast<double> (scalar)); }
 
   FloatNDArray float_array_value (bool = false) const
-    { return FloatNDArray (dim_vector (1, 1), scalar); }
+    { return FloatNDArray (dim_vector (1, 1), static_cast<double> (scalar)); }
 
   Complex complex_value (bool = false) const { return scalar; }
 

[John W. Eaton]

On 30-Jun-2008, David Bateman wrote:

| John W. Eaton wrote:
| > On 26-Jun-2008, David Bateman wrote:
| >
| > | consider the attached changeset instead.
| >
| > It seems odd to me that the octave_value constructor that takes an
| > Array<octave_idx_type> value would convert it to NDArray internally
| > instead of storing it as an Array<octave_idx_type>.  The other
| > conversions also seem odd, but I guess we already have a few other
| > constructors that have similar options.  What about defining
| > Array<octave_idx_type> to NDArray conversion with the options and then
| > using the existing octave_value (const NDArray&) constructor instead?
| > It seems to me that this approach might cause less trouble later if we
| > decided we wanted an actual Array<octave_idx_type> value in the
| > octave_value hierarchy.
| >
| > jwe
| >  
| Ok, then here is take 3.

I applied it, but removed the function

  NDArray (const ArrayN<octave_idx_type>& a, bool zero_based = false,
           bool negative_to_nan = false);

since ArrayN is derived from Array, so I think we only need the
version that takes an Array argument.

Thanks,

jwe