Re: Empty matrices don't have trailing ones?

[John W. Eaton]

On 10-May-2012, Jordi Gutiérrez Hermoso wrote:

| On 10 May 2012 00:46, Jordi Gutiérrez Hermoso <address@hidden> wrote:
| > I'm trying to figure out the rationale for having dim_vector::redim
| > padding all-zero dim_vectors with zeros instead of ones.
| 
| Responding to myself, looks like many tests break if dim_vector::redim
| always pads with ones, but the two problems I've encountered like bug
| #33216 and x = []; x(:,:,1) do get fixed by padding with ones. I won't
| try to investigate the logic, so instead I attempted the attached
| patch which disables zero-padding for the two cases in which I think
| it doesn't make sense.
| 
| I think this breaks ABI or API, and the bug it's patching is very
| obscure and minor, so I am thinking about pushing this on the default
| branch. Thoughts?

How about the following change instead?  With it, I don't see any
additional test failures.  Probably it needs a test for the reported
bug and to have the commit message include the bug report number.

Also, although it doesn't change the API and there are no new failing
tests, I'm not sure whether it is safe for stable because I don't know
whether there might be other parts of Octave that depend on the
previous behavior of appending zero to all-zero dim vectors and one to
others.

jwe

# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1336684014 14400
# Node ID 4a206cb800523d19c3fb3bd98e150280b4aaa3fb
# Parent  13cc11418393d8b74c67501227f1057045f5b125
when redimensioning, always pad dim_vector objects with 1

* dim-vector.cc (dim_vector::redim): Always pad with 1.
* dim-vector.h (dim_vector::redim): Update comment.
* Array.cc (Array<T>::assign): Query dimensions for all zeros before
redimensioning.

diff --git a/liboctave/Array.cc b/liboctave/Array.cc
--- a/liboctave/Array.cc
+++ b/liboctave/Array.cc
@@ -1165,16 +1165,21 @@
 Array<T>::assign (const idx_vector& i, const idx_vector& j,
                   const Array<T>& rhs, const T& rfv)
 {
+  bool initial_dims_all_zero = dimensions.all_zero ();
+
   // Get RHS extents, discarding singletons.
   dim_vector rhdv = rhs.dims ();
+
   // Get LHS extents, allowing Fortran indexing in the second dim.
   dim_vector dv = dimensions.redim (2);
+
   // Check for out-of-bounds and form resizing dimensions.
   dim_vector rdv;
+
   // In the special when all dimensions are zero, colons are allowed
   // to inquire the shape of RHS.  The rules are more obscure, so we
   // solve that elsewhere.
-  if (dv.all_zero ())
+  if (initial_dims_all_zero)
     rdv = zero_dims_inquire (i, j, rhdv);
   else
     {
@@ -1268,6 +1273,8 @@
     assign (ia(0), ia(1), rhs, rfv);
   else if (ial > 0)
     {
+      bool initial_dims_all_zero = dimensions.all_zero ();
+
       // Get RHS extents, discarding singletons.
       dim_vector rhdv = rhs.dims ();
 
@@ -1280,7 +1287,7 @@
       // In the special when all dimensions are zero, colons are
       // allowed to inquire the shape of RHS.  The rules are more
       // obscure, so we solve that elsewhere.
-      if (dv.all_zero ())
+      if (initial_dims_all_zero)
         rdv = zero_dims_inquire (ia, rhdv);
       else
         {
diff --git a/liboctave/dim-vector.cc b/liboctave/dim-vector.cc
--- a/liboctave/dim-vector.cc
+++ b/liboctave/dim-vector.cc
@@ -272,16 +272,11 @@
     {
       dim_vector retval = alloc (n);
 
-      int pad = 0;
       for (int i = 0; i < n_dims; i++)
-        {
-          retval.rep[i] = rep[i];
-          if (rep[i] != 0)
-            pad = 1;
-        }
+        retval.rep[i] = rep[i];
 
       for (int i = n_dims; i < n; i++)
-        retval.rep[i] = pad;
+        retval.rep[i] = 1;
 
       return retval;
     }
diff --git a/liboctave/dim-vector.h b/liboctave/dim-vector.h
--- a/liboctave/dim-vector.h
+++ b/liboctave/dim-vector.h
@@ -381,8 +381,7 @@
   // Force certain dimensionality, preserving numel ().  Missing
   // dimensions are set to 1, redundant are folded into the trailing
   // one.  If n = 1, the result is 2d and the second dim is 1
-  // (dim_vectors are always at least 2D).  If the original dimensions
-  // were all zero, the padding value is zero.
+  // (dim_vectors are always at least 2D).
 
   dim_vector redim (int n) const;