[Octave-bug-tracker] [bug #52904] "mesh" with input array of "logical" causes error

[Dan Sebald]

Follow-up Comment #4, bug #52904 (project octave):

OK, a bit trickier than I imagined.  Let me explain what the issues are, i.e.,
why the crash in 4.2.1 and why the strange error message.  (And I'll probably
conclude it is best to handle this by adding a warning message in surface...or
do the conversion from logical to something else.)

So, let me start by illustrating that Octave is doing as expected in a way. 
"logical" is not a valid class for the zdata property.  I'm not considering
the conversion; what I mean is that we can't have "ans" be "logical" in the
following:


octave:20> h = mesh(zeros(25));
octave:21> class(get(h,'zdata'))
ans = double


But let me try to force that to happen after having created the valid surface
object:


octave:22> set(h,'zdata',zeros(25)==0)
error: invalid value for array property "zdata"


OK, that's well and good, as far as expectations go given that "zdata"
property can't be logical.

But that creates a conundrum.  If we convert zdata, i.e., this:

h = mesh(zeros(25)==0);

internally to uint8 (or whatever) prior to continuing on to __go_surface__, we
still can't set the zdata property with logical data.  In other words,
"logical" is fine at the input, but not directly--kind of strange.

Now the more difficult issue, and why I suspect 4.2.1 crashes.  Notice in the
above bad set() command that there is a nice clean error message.  That error
message does in fact occur if I set the input of the mesh() routine to logical
(I'm going to write in the dump below where I think the problematic result
is):


octave:22> h = mesh(zeros(25)==0);
error: invalid value for array property "zdata"
error: called from
    surface>__surface__ at line 198 column 5
    surface at line 63 column 19
    mesh at line 77 column 12
[HERE'S THE PROBLEM ERROR MESSAGE]
error: __gnuplot_draw_axes__: invalid grid data
error: called from
    __gnuplot_draw_axes__ at line 1249 column 11
    __gnuplot_draw_figure__ at line 164 column 17
    __gnuplot_drawnow__ at line 86 column 5
octave:22> 


There are two methods here:

1) VALIDATE: There is an array_property::validate (const octave_value& v) in
graphics.cc.  That's where the "error: invalid value for array property
"zdata"" comes from.  It's very generic, and of course it is in __go_surface__
routine where this happens.  [We could add extra checks in the surface()
routine and issue an error prior to calling __go_surface__ and not continue,
as an option.]

2) ZDATA (ET AL.) UPDATE: The way that this drawing and re-drawing works is
through a listener-like mechanism, update_data, update_xdata, update_ydata,
update_zdata, etc.  Here's an example of the surface's routine from
graphics.in.h:


    void update_zdata (void)
    {
      update_vertex_normals ();
      set_zlim (zdata.get_limits ());
    }


So, that's where the problem lies.  Even though the mesh()/surface() is
failing as it is designed to do for "logical" class data, for some reason the
callback method is attempting a redraw.  Exactly how that comes about I don't
know.  Does the surface graphics object continue to reside in memory?  Is it
bogus memory that the callback is operating on, sure to be prone to program
faults?  Anyway, such a thing just shouldn't happen.  Note in the graphics.cc
file is a FIXME construct to prevent such mismatched xdata, ydata, zdata
errors:


void
surface::properties::update_vertex_normals (void)
{
  if (vertexnormalsmode_is ("auto"))
    {
      Matrix x = get_xdata ().matrix_value ();
      Matrix y = get_ydata ().matrix_value ();
      Matrix z = get_zdata ().matrix_value ();

      int p = z.columns ();
      int q = z.rows ();

      // FIXME: There might be a cleaner way to do this.  When data is
changed
      // the update_xdata, update_ydata, update_zdata routines are called in
a
      // serial fashion.  Until the final call to update_zdata the matrices
      // will be of mismatched dimensions which can cause an out-of-bound
      // indexing in the code below.  This one-liner prevents calculating
      // normals until dimensions match.
      if (x.columns () != p || y.rows () != q)
        return;

[SNIP]
    }
}


In summary, I'm not quite sure why the failed mesh("logical") routine is
continuing on after the error message to incur a second error message during
the redraw in the graphics-toolkit.  One would think the above test would
prevent that (of course, after that initial error, can we expect normal
program flow and tests to function expectedly?).

The following works reasonably:


octave:1> x = [1:25];
octave:2> y = [1:25]';
octave:3> z = zeros(27);
octave:4> mesh(x,y,z)
error: surface: rows (Z) must be the same as length (Y) and columns (Z) must
be the same as length (X)
error: called from
    surface>__surface__ at line 131 column 11
    surface at line 63 column 19
    mesh at line 77 column 12


but that's because this test is done in __surface__ in surface.m *prior* to
the __go_surface__ function that creates a surface graphics object.  And the
following is a graceful error as well:


octave:4> z = zeros(25);
octave:6> h = mesh(x,y,z)
octave:7> set(h,'zdata',zeros(27))
error: __gnuplot_draw_axes__: invalid grid data
error: called from
    __gnuplot_draw_axes__ at line 1249 column 11
    __gnuplot_draw_figure__ at line 164 column 17
    __gnuplot_drawnow__ at line 86 column 5


but that is likely because the surface object still resides in memory as a
valid object in the example.  Whereas, when we attempt such a thing at
instantiation and there is an error, the surface object is probably deleted
while the callback mechanisms are active.

I really don't feel like digging into that C++ code; rather covering this with
plaster by adding some tests or conversions to suface() is the easy way out. 
On the other hand, if I were to guess, I'd say that the graphics objects
constructors shouldn't activate their callback mechanisms, i.e., the
update_xyzcdata(), until all else passes without error.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?52904>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/