patchGNU Octave - Patches: patch #8307, Add new ordschur function.

 
 

patch #8307: Add new ordschur function.

Submitter:  Sébastien Villemot <svillemot>
Submitted:  Wed 05 Feb 2014 05:23:40 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  mtmiller Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 05 Feb 2016 11:10:44 AM UTC, comment #8: 

Dear all,

I tried to implement the last syntax of matlab's ordschur. My approach was to convert the vector CLUSTER into a sequence of SELECT vectors. It seems to work, even if sometimes the result is different from Matlab (but correct in terms of U*T*U'=US*TS*US').
The files (sortcluster.m and testordschur.m) are written not following Octave standards, in order to be run in Matlab. This is probably not the best way to do it, but at least it is a way.

(file #36252, file #36253)

Marco Caliari <caliari>
Group Member
Wed 18 Feb 2015 06:22:20 PM UTC, comment #7: 

Thank you for the patch. I have pushed it, see http://hg.savannah.gnu.org/hgweb/octave/rev/56157a7505ed


Carnë Draug <carandraug>
Group Member
Sat 07 Feb 2015 08:54:53 PM UTC, comment #6: 

Thanks Mike for your review. And thanks Carnë for improving my initial submission.

I attach a new version of the patch that incorporates Carnë's improvements and that moves the function to corefcn.

I don't think it makes much sense to put the heart of the function into liboctave, since ordschur is just a thin wrapper around LAPACK's trsen (which does all the real computations).


(file #33027)

Sébastien Villemot <svillemot>
Fri 06 Feb 2015 06:38:09 PM UTC, comment #5: 

I have attached a patch that applies on top of the original. It drops the use OCTAVE_LOCAL_BUFFER and instead initializes octave dataypes and then uses fortran_vec() to pass a pointer to the fortran routines. It makes a bunch of other smaller changes as well.

(file #33019)

Carnë Draug <carandraug>
Group Member
Fri 06 Feb 2015 02:25:46 PM UTC, comment #4: 

Thanks for the patch. I just took a very quick glance at this and it looks mostly complete. Includes NEWS, doc changes, test cases, all good.

Is there a reason for putting this function in dldfcn? I think that is usually used for functions that load larger external libraries that we don't want linked into Octave all the time or that may be optional. This is just wrapping LAPACK functions. Also the schur function is in corefcn. I think this should be moved there also.

See also the thread at http://octave.1599824.n4.nabble.com/Numerical-or-other-significant-code-in-DEFUN-functions-td4652573.html. Jwe would prefer that numerical functions can be called in liboctave and the interpreter binding be a wrapper around that. I'm not sure where the line is or whether this function needs to be in liboctave or not. At the very least, this function is a much thinner wrapper, just dispatching the given matrix arguments to the appropriate LAPACK functions.

Mike Miller <mtmiller>
Group Member
Thu 05 Feb 2015 10:31:02 PM UTC, comment #3: 

Here is an updated patch that applies cleanly to the current default branch.

(file #33010)

Sébastien Villemot <svillemot>
Thu 05 Feb 2015 03:00:08 PM UTC, comment #2: 

Any chance to have this patch review and applied before the 4.0 release?

Thanks

Sébastien Villemot <svillemot>
Wed 05 Feb 2014 05:24:48 PM UTC, comment #1: 

I forgot to mention that this patch should be suitable for the gui-release branch, since it does not break any existing API.

Sébastien Villemot <svillemot>
Wed 05 Feb 2014 05:23:40 PM UTC, original submission:  

Note that the patch implements only the first syntax of the MATLAB equivalent function.

I plan to add the other syntaxes in subsequent patches.

Sébastien Villemot <svillemot>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36252:  sortcluster.m added by caliari (726B - application/binary)
file #36253:  testordschur.m added by caliari (1KiB - application/binary)
file #33027:  ordschur-corefcn.patch added by svillemot (12KiB - text/x-diff)
file #33019:  trsen_less_C.path added by carandraug (9KiB - application/octet-stream)
file #33010:  ordschur4.0.patch added by svillemot (12KiB - text/x-diff - Updated patch ready for 4.0 release)
file #30463:  ordschur.patch added by svillemot (12KiB - 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 caliari (Updated the item)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by svillemot (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 logged-in users can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-05 caliari Attached File- Added sortcluster.m, #36252
        Attached File- Added testordschur.m, #36253
    2015-02-18 carandraug StatusNeed Info Done
        Open/ClosedOpen Closed
    2015-02-07 svillemot Attached File- Added ordschur-corefcn.patch, #33027
    2015-02-06 carandraug Attached File- Added trsen_less_C.path, #33019
    2015-02-06 mtmiller StatusNone Need Info
        Assigned toNone mtmiller
    2015-02-05 svillemot Attached File- Added ordschur4.0.patch, #33010
    2014-02-05 svillemot Attached File- Added ordschur.patch, #30463

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code