[Octave-bug-tracker] [bug #43925] isscalar behaves incorrectly for some user-defined classes

[Julien Bect]

Follow-up Comment #25, bug #43925 (project octave):

Ok, so is it a regression (between 3.8.2 and 4.0.x) or not?

[thinking....]

-----

First element: in my initial post I wrote:

> In Octave 3.8.2, isscalar(x) returns true if numel(x) is 1.

Actually, it seems that both 3.8.2 and 4.0.x define isscalar(x) as (numel(x)
== 1), but in 3.8.2 this is in an m-file, whereas in 4.0.x this in C++.

So, I agree that in term of internal implementation, isscalar seems to have
been wrong, in principle, all the time.

-----

Second: the main case for a regression came, IIRC, from a comment by Oliver
Heimlich (initial post of bug #44498):

> In GNU Octave 3.8.2 the overriden size had been respected by isscalar,
isvector, and isequal.

I don't think I ever checked myself what was going on in 3.8.2.

-----

Thinking about Oliver's comment:

a) The case of isequal different, and has already been dealt with in bugs
#44334 and #45118, so let's forget about it.

b) The case of isvector is the subject of bug #44498.  In 3.8.x in was an
m-file implementation calling size (which is ok) and ndims (which is not ok). 
So, I would say that the behaviour was ok for two-dimensional arrays and wrong
for higher-dimensional arrays.  In 4.0.x, issvector is implemented in C++ and
used ndims(), so it _never_ respects the oveloaded size. Summary for the case
of issvector: *for isvector, we have a Matlab-compatibility regression in the
case of two-dimensional arrays*.

c) The case of isscalar still puzzles me.  Given my remark above, I now think
It has been wrong all the time, being based on numel. The only explanation
that I can think of is that Oliver had numel overloaded in his test class,
equal to prod (size (x)).

-----

About some other functions:

d) rows, columns: The behaviour hasn't changed. Overloaded size has never been
respected. Note that these functions do not exist in Matlab.  Therefore, as
far as they are concerned, Matlab-compatibility is not an issue.  Only
consistency with the other functions is.

e) isrow, iscolumn: Same case as isvector. 3.8.x had an m-file implementation
based on size and ndims => *Matlab-compatiblity regression for two-dimensional
arrays*.

f) issquare : 3.8.x also had an m-file implementation based on size and ndims,
which thus correctly respected overloaded size functions in the case of
two-dimensional arrays.  Now it doesn't respect it at all.  This is a *change
of behaviour*, but not a Matlab-compatinility issue since the function does
not exist in Matlab.

g) ismatrix: Very different case.  Was not at all Matlab-compatible in 3.8.x
(this has been discussed elsewhere).  The 4.0.x version is better in terms of
Matlab compatiblity, but ignores the overloaded size.

-----

Summary: I believe that

 * we have several regressions between 3.8.2 and 4.0.x wrt supporting
overloaded size: isvector, isrow, iscolumn.

 * fixing these functions but not the others would be a bad idea for two
reasons: 1) Even they have not regressed, the other functions are faulty in
terms of Matlab-compatibility, and 2) all these size-related functions should
behave similarly.

-----

Sorry for the very long post.

Now, the decisions to be made is: do we fix this on stable, on default, or do
we fix the regressions only on stable and the rest of it on default ?

I would vote for fixing everything on stable...

    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  Message posté via/par Savannah
  http://savannah.gnu.org/