bugGNU Octave - Bugs: bug #42422, ismatrix: Matlab is only true for...

 
 

bug #42422: ismatrix: Matlab is only true for 2 dimensions

Submitter:  Carnë Draug <carandraug>
Submitted:  Sun 25 May 2014 07:43:13 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  None Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 13 Feb 2015 12:46:54 PM UTC, comment #14: 

Just an idea, should the C++ is_matrix_type method match the ismatrix function? Do we have ismatrix to mean different things?

Carnë Draug <carandraug>
Group Member
Fri 13 Feb 2015 07:27:26 AM UTC, comment #13: 

Done.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 12 Feb 2015 10:37:37 PM UTC, comment #12: 

You're right, I'll add an entry tomorrow. I think it is the right moment in time to do this change just before the next big release.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 12 Feb 2015 10:33:42 PM UTC, comment #11: 

Such a change breaks backwards compatibility, the function does something quite different now. This should be mentioned on the NEWS file.

Carnë Draug <carandraug>
Group Member
Thu 12 Feb 2015 05:39:16 PM UTC, comment #10: 

I took the effort to make ismatrix() Matlab compatible now, see changeset 00e31f316a3a.

Kai Torben Ohlhus <siko1056>
Group Member
Wed 28 May 2014 04:24:00 PM UTC, comment #9: 

It is pretty clear that Matlab only uses ismatrix() for checking array dimensions while Octave also tests that the argument is a "numeric" class which includes numeric, logical, and character arrays.

A trivial implementation of ismatrix to match Matlab is


retval = (ndims (x) == 2);


But, as I expected, a lot of Octave core code is relying on also checking the class of the argument.  Substituting the new ismatrix and running 'make check' results in 47 new test failures.

The closest existing function is isreal() which returns true for numeric, logical, and character matrices.  It would mean tracking down each of the 47 test failures and verifying that isreal could be substituted there.  Alternatively, we could create a new function, some is_close_enough_to_numeric_for_government_work() which includes numeric, logical, and char types, and replace calls with that new function.

As for performance, I'll file a separate issue report about promoting some of the frequently used predicate tests into C++ functions.  There is already a patch report about doing that for isscalar but it should probably be generalized to include all of the frequently used input validation tests.

Rik <rik5>
Group administrator
Wed 28 May 2014 04:00:38 PM UTC, comment #8: 

Results from ismatrix testing:


For MATLAB R2013b the result is:

>> ismatrix ({1,2;3,4})
ans =
     1

>> [x(1:2,1:2).a] = deal (1,1,1,1);
>> ismatrix (x)
ans =
     1

>> ismatrix ([])
ans =
     1

>> ismatrix (zeros (10, 0))
ans =
     1

>> ismatrix (zeros (0,10))
ans =
     1

>> x = zeros (0,10,0,0,0)
x =
   Empty array: 0-by-10-by-0-by-0-by-0

>> size (x)
ans =
     0    10     0     0     0

>> ndims (x)
ans =
     5

>> ismatrix (x)
ans =
     0

>> x = zeros (0,10,0,1,0)
x =
   Empty array: 0-by-10-by-0-by-1-by-0

>> ismatrix (x)
ans =
     0

>> isscalar ({1})
ans =
     1

>> isvector ([])
ans =
     0

>> isvector (1)
ans =
     1

>> isvector (zeros (1,0))
ans =
     1

>> isvector (cell (1,3))
ans =
     1


Rik <rik5>
Group administrator
Tue 27 May 2014 07:02:41 PM UTC, comment #7: 

Definitely try to find someone with access
to recent Matlab.

I do not have direct access, but could possibly find
someone who does.

Michael Godfrey <godfrey>
Group Member
Tue 27 May 2014 05:22:43 PM UTC, comment #6: 

Is it worth posting the test cases on the octave-maintainers list to get a quick response?  I'm not sure anyone following this bug report has a new enough copy of Matlab to run the tests.

Rik <rik5>
Group administrator
Tue 27 May 2014 04:10:53 PM UTC, comment #5: 

It seems to me that the really simple predicate functions should maybe all be internal C++ functions.  Otherwise, the overhead of the function call kills performance, and many of these predicates are called when doing argument checking, so it would probably be good for them to be as fast as possible.  They are simple functions, so writing and maintaining them in C++ should be pretty easy.

John W. Eaton <jwe>
Group administrator
Mon 26 May 2014 04:11:10 PM UTC, comment #4: 

We should also verify with a recent version of Matlab what the true behavior of all these commands are.  The documentation now makes isscalar, isrow, iscolumn, isvector, ismatrix suggest that they are only concerned with the return value from size().  ismatrix() on Octave, however, also checks that the array is numeric.  Finally, ismatrix is implemented in C++ while the others are in m-files.  This is really a simple function and should probably be made an m-file.

Some tests for a recent version of Matlab:


ismatrix ({1,2;3,4})
[x(1:2,1:2).a] = deal (1,1,1,1);
ismatrix (x)
ismatrix ([])
ismatrix (zeros (10, 0))
ismatrix (zeros (0,10))
x = zeros (0,10,0,0,0)
size (x)
ndims (x)
ismatrix (x)
isscalar ({1})
isvector ([])
isvector (1)
isvector (zeros (1,0))
isvector (cell (1,3))


Rik <rik5>
Group administrator
Mon 26 May 2014 02:45:04 PM UTC, comment #3: 

Also, the current ismatrix behavior is new
in 3.8.0, so there is not a lot of history.

NEWS.3 discusses this and issquare and issymmetric.
It does not appear that there is a problem with
issquare and issymetric. They act on matrices.

Michael Godfrey <godfrey>
Group Member
Mon 26 May 2014 02:29:12 PM UTC, comment #2: 

This raises some issues: It is common
practise to use the word array for
multidimensional arrangements of elements.
Thus, the notation:

array(arguments)

The number of arguments is termed the
dimension of the array.

Matrix is usually used to mean an array of
2 arguments.  It is not usual practise to
use "matrix" for an array of other than
2 arguments.

Octave (and the Manual) follow this nomenclature
except for the behavior of ismatrix. (As far as
I know)

Thus, does everyone agree that, in priciple,
ismatrix should have been named isarray?

This leads to the suggestion that isarray should
be implemented, and should behave as ismatrix
does now. This would allow (a possibly more
managable) editing of current ismatrix references
to isarray where appropriate.

Then the question of at some point making ismatrix
valid only for matrices (like Matlab) could
be handled with less upsets.

Michael Godfrey <godfrey>
Group Member
Mon 26 May 2014 05:03:13 AM UTC, comment #1: 

I guess we could change the definition and see how many tests fail using 'make check'.  My guess is that the number is going to be huge.  There are a lot of places where ismatrix is used just to check that we have a numeric array, as opposed to a cell or struct.

Rik <rik5>
Group administrator
Sun 25 May 2014 07:43:13 PM UTC, original submission:  

It seems that Matlab behaviour of this function is different from Octave. See this bug report for matlab2tikz:

https://github.com/nschloe/matlab2tikz/issues/143

This function has been part of Octave since at least 2002 while according to

http://www.dynare.org/DynareWiki/MatlabVersionsCompatibility

Matlab only added it in 2010b.

Changing for Matlab compatibility should be pretty simple but how to deal with breaking backwards compatibility?

Carnë Draug <carandraug>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by carandraug (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-12 siko1056 Open/ClosedOpen Closed
    2014-05-28 rik5 Dependencies- bugs #42454 is dependent

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code