x(idx)=y(idx) for empty idx

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

x(idx)=y(idx) for empty idx

Paul Kienzle
Consider:

        y = rand(100);
        idx = y>1;
        y(idx) = y(idx)-1;

With warn_fortran_indexing=1, this prints:

        warning: A(idx) = []: expecting A to be row or column vector or scalar

As a result we have a bunch of code like this:

        idx = condition;
        if any ( idx (:))
            A(idx) = f(A(idx));
        end

instead of:

        idx = condition;
        A(idx) = f(A(idx));


For dimension != 2, the simple code seg faults (but only
for empty idx):

   octave:1> y = rand(10,10,10);
   octave:2> idx = y>1;
   octave:3> y(idx) = y(idx)-1;
   panic: Segmentation fault -- stopping myself...

System: Debian, with CVS download from 2004-03-02.

Paul Kienzle
[hidden email]


Reply | Threaded
Open this post in threaded view
|

x(idx)=y(idx) for empty idx

John W. Eaton-6
On  3-Mar-2004, Paul Kienzle <[hidden email]> wrote:

| For dimension != 2, the simple code seg faults (but only
| for empty idx):
|
|    octave:1> y = rand(10,10,10);
|    octave:2> idx = y>1;
|    octave:3> y(idx) = y(idx)-1;
|    panic: Segmentation fault -- stopping myself...

Please try the following patch.

Thanks,

jwe


liboctave/ChangeLog:

2004-03-05  John W. Eaton  <[hidden email]>

        * Array.cc (Array<T>::maybe_delete_elements): Return immediately
        if all LHS dimensions are zero.  For one index case, freeze and
        sort idx_vec before checking length, and do nothing if
        num_to_delete is zero.



Index: liboctave/Array.cc
===================================================================
RCS file: /usr/local/cvsroot/octave/liboctave/Array.cc,v
retrieving revision 1.107
diff -u -r1.107 Array.cc
--- liboctave/Array.cc 2 Mar 2004 05:12:32 -0000 1.107
+++ liboctave/Array.cc 5 Mar 2004 16:16:52 -0000
@@ -1540,6 +1540,9 @@
 
   dim_vector lhs_dims = dims ();
 
+  if (lhs_dims.all_zero ())
+    return;
+
   int n_lhs_dims = lhs_dims.length ();
 
   Array<int> idx_is_colon (n_idx, 0);
@@ -1791,60 +1794,68 @@
  }
       else if (n_idx == 1)
  {
-  // This handle cases where we only have one index (not colon).
-  // The index denotes which elements we should delete in the array
-  // which can be of any dimension. We return a column vector, except
-  // for the case where we are operating on a row column. The elements
-  // are numerated columns by column.
+  // This handle cases where we only have one index (not
+  // colon).  The index denotes which elements we should
+  // delete in the array which can be of any dimension. We
+  // return a column vector, except for the case where we are
+  // operating on a row vector. The elements are numerated
+  // column by column.
   //
   // A(3,3,3)=2;
   // A(3:5) = []; A(6)=[]
-  //
+
+  int lhs_numel = numel ();
+
   idx_vector idx_vec = ra_idx(0);
 
-  int num_to_delete = idx_vec.capacity ();
+  idx_vec.freeze (lhs_numel, 0, true, liboctave_wrore_flag);
+      
+  idx_vec.sort (true);
 
-  int lhs_numel = numel ();
+  int num_to_delete = idx_vec.length (lhs_numel);
 
-  int new_numel = lhs_numel - num_to_delete;
+  if (num_to_delete > 0)
+    {
+      int new_numel = lhs_numel - num_to_delete;
 
-  T *new_data = new T[new_numel];
+      T *new_data = new T[new_numel];
 
-  Array<int> lhs_ra_idx (ndims (), 0);
+      Array<int> lhs_ra_idx (ndims (), 0);
 
-  int ii = 0;
-  int iidx = 0;
+      int ii = 0;
+      int iidx = 0;
 
-  for (int i = 0; i < lhs_numel; i++)
-    {
-      if (iidx < num_to_delete && i == idx_vec.elem (iidx))
- {
-  iidx++;
- }
-      else
+      for (int i = 0; i < lhs_numel; i++)
  {
-  new_data[ii++] = elem (lhs_ra_idx);
- }
+  if (iidx < num_to_delete && i == idx_vec.elem (iidx))
+    {
+      iidx++;
+    }
+  else
+    {
+      new_data[ii++] = elem (lhs_ra_idx);
+    }
 
-      increment_index (lhs_ra_idx, lhs_dims);
-    }
+  increment_index (lhs_ra_idx, lhs_dims);
+ }
 
-  if (--(Array<T>::rep)->count <= 0)
-    delete Array<T>::rep;
+      if (--(Array<T>::rep)->count <= 0)
+ delete Array<T>::rep;
 
-  Array<T>::rep = new typename Array<T>::ArrayRep (new_data, new_numel);
+      Array<T>::rep = new typename Array<T>::ArrayRep (new_data, new_numel);
 
-  dimensions.resize (2);
+      dimensions.resize (2);
 
-  if (lhs_dims.length () == 2 && lhs_dims(1) == 1)
-    {
-      dimensions(0) = new_numel;
-      dimensions(1) = 1;
-    }
-  else
-    {
-      dimensions(0) = 1;
-      dimensions(1) = new_numel;
+      if (lhs_dims.length () == 2 && lhs_dims(1) == 1)
+ {
+  dimensions(0) = new_numel;
+  dimensions(1) = 1;
+ }
+      else
+ {
+  dimensions(0) = 1;
+  dimensions(1) = new_numel;
+ }
     }
  }
       else if (num_ones (idx_is_colon) < n_idx)


Reply | Threaded
Open this post in threaded view
|

x(idx)=y(idx) for empty idx

John W. Eaton-6
In reply to this post by Paul Kienzle
On  3-Mar-2004, Paul Kienzle <[hidden email]> wrote:

| Consider:
|
| y = rand(100);
| idx = y>1;
| y(idx) = y(idx)-1;
|
| With warn_fortran_indexing=1, this prints:
|
| warning: A(idx) = []: expecting A to be row or column vector or scalar
|
| As a result we have a bunch of code like this:
|
| idx = condition;
| if any ( idx (:))
|    A(idx) = f(A(idx));
| end
|
| instead of:
|
| idx = condition;
| A(idx) = f(A(idx));

Please try the following patch.  It eliminates the warning you see
above and also fixes a bug (if the warning flag was set, deleting
elements didn't work at all).

You will still see a warning about "single index used for matrix" in
the 2-d case.  For N-d arrays, there is no warning.

Yes, we should be consistent here.  I hope that all the different
indexing and indexed assignment functions will eventually be combined
so we only have one of each.  But first, I think the N-d versions
could use some more work.

Thanks,

jwe


2004-03-05  John W. Eaton  <[hidden email]>

        * Array.cc (Array<T>::maybe_delete_elements_2):
        Omit Fortran-indexing warning.


Index: liboctave/Array.cc
===================================================================
RCS file: /usr/local/cvsroot/octave/liboctave/Array.cc,v
retrieving revision 1.108
diff -u -r1.108 Array.cc
--- liboctave/Array.cc 5 Mar 2004 16:19:11 -0000 1.108
+++ liboctave/Array.cc 5 Mar 2004 16:34:03 -0000
@@ -1270,14 +1270,6 @@
       n = nr * nc;
       nr = 1;
       nc = n;
-
-      if (liboctave_wfi_flag)
- {
-  (*current_liboctave_warning_handler)
-    ("A(idx) = []: expecting A to be row or column vector or scalar");
-
-  return;
- }
     }
 
   if (idx_arg.is_colon_equiv (n, 1))