bugGNU Octave - Bugs: bug #42688, isdiag, isbanded, istril, istriu...

 
 

bug #42688: isdiag, isbanded, istril, istriu functions missing in Octave

Submitter:  Massimiliano Fasi <mfasi>
Submitted:  Sat 05 Jul 2014 08:26:20 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed 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

Tue 08 Jul 2014 12:32:56 PM UTC, comment #8: 

Thank you. I was already working on the bandwidth function but wanted some feedback on the easiest ones before submitting. I've opened another bug (https://savannah.gnu.org/bugs/?42703) on the topic and submitted a patch.

Massimiliano Fasi <mfasi>
Mon 07 Jul 2014 06:08:28 PM UTC, comment #7: 

Thanks for checking.  I'll close this report now.

If you're feeling inspired, the bandwidth function is probably also easy to implement in an m-file.  It seems that the function was added in 2014a along with isbanded, istril, istriu, and isdiag.

Rik <rik5>
Group administrator
Mon 07 Jul 2014 05:55:01 PM UTC, comment #6: 

I've just asked on IRC.


>> isdiag (char ([65, 0; 0, 65]))
Undefined function 'isdiag' for input arguments of type 'char'.

>> A = cell (2,2); A(1,1) = 1; A(2,2) = 2; isdiag (A)
Conversion to cell from double is not possible.



Massimiliano Fasi <mfasi>
Mon 07 Jul 2014 05:16:09 PM UTC, comment #5: 

@Rik: Thank you. Unfortunately, I cannot run such tests as I don't have access to the latest Matlab version, where these functions were introduced.

Massimiliano Fasi <mfasi>
Mon 07 Jul 2014 04:19:11 PM UTC, comment #4: 

@Massimliano: I committed your changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/49961d67e4b9).  I changed '~' to '!' for logical not as that is the Octave convention.  I also removed the third return argument from find() function calls since you weren't using it.  I changed isnumeric() to isreal() when checking whether LOWER and UPPER are acceptable inputs to isbanded().

Do you have access to Matlab to run a few more tests?  Right now we are checking whether the matrix is numeric or logical.  Often Matlab will accept character matrices as well.  Does this code return true?


isdiag (char ([65, 0; 0, 65]))


Also, if the test is only for structure Matlab often accepts cell arrays as well.  What does Matlab return for


A = cell (2,2);
A(1,1) = 1;
A(2,2) = 2;
isdiag (A)


Rik <rik5>
Group administrator
Mon 07 Jul 2014 03:56:37 PM UTC, comment #3: 

I think it is okay to leave these as m-files.  The functions like isvector were promoted because they are used so frequently in input validation.  I think these will be used less frequently and so a few milliseconds penalty for being an interpreted file is fine.


Rik <rik5>
Group administrator
Mon 07 Jul 2014 01:39:03 PM UTC, comment #2: 

Hi Pantxo,

thank you for your feedback. I've submitted a patch that addresses the second and third bullets of your list. I'm not sure about the first, as the other matrix structure test functions are m-files located in scripts/linear-algebra (issymmetric, ishermitian).

(file #31686)

Massimiliano Fasi <mfasi>
Mon 07 Jul 2014 10:36:53 AM UTC, comment #1: 

Hi Massimiliano,

I have a few remarks:

  • some shape validations functions like isvector, isrow ... were recently translated from .m to C++ for performance (see changeset http://hg.savannah.gnu.org/hgweb/octave/rev/4f0e4f20a33f). I wonder if the new ones you propose shouldn't have the same performance constraint. Anyways, a preliminary (or definite) .m implementation is valuable
  • the new functions should eventually return true only for 2 dimensional matrices (see "ndims")
  • in order to return a boolean, a cleaner approach would be using "all" in place of "min".
Pantxo Diribarne <pantxo>
Group Member
Sat 05 Jul 2014 08:26:20 AM UTC, original submission:  

MATLAB 8.3 (R2014a) introduces some new functions to test matrix structure. The patch I attach adds isdiag, isbanded, istril and istriu.

Massimiliano Fasi <mfasi>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #31686:  structure.patch added by mfasi (8KiB - text/x-diff)
file #31678:  structure.patch added by mfasi (8KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by mtmiller (Updated the item)
  • -email is unavailable- added by mfasi (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-07-07 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2014-07-07 rik5 StatusPatch Submitted In Progress
    2014-07-07 mfasi Attached File- Added structure.patch, #31686
    2014-07-05 rik5 SummaryBunch of functions for testing matrix structure missing in Octave isdiag, isbanded, istril, istriu functions missing in Octave
    2014-07-05 mtmiller StatusNone Patch Submitted
    2014-07-05 mfasi Attached File- Added structure.patch, #31678

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code