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

 
 

patch #8307: Add new ordschur function.

Submitted by:  Sébastien Villemot <svillemot>
Submitted on:  Wed Feb 5 17:23:40 2014  
 
Category: NonePriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Mike Miller <mtmiller>Open/Closed: Closed

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri Feb 5 11:10:44 2016, 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 UTU'=USTSUS').
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>
Project Member
Wed Feb 18 18:22:20 2015, comment #7:

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

Carnë Draug <carandraug>
Project Member
Sat Feb 7 20:54:53 2015, 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 Feb 6 18:38:09 2015, 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>
Project Member
Fri Feb 6 14:25:46 2015, 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>
Project AdministratorIn charge of this item.
Thu Feb 5 22:31:02 2015, comment #3:

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

(file #33010)

Sébastien Villemot <svillemot>
Thu Feb 5 15:00:08 2015, comment #2:

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

Thanks

Sébastien Villemot <svillemot>
Wed Feb 5 17:24:48 2014, 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 Feb 5 17:23:40 2014, 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 File(s):
   
   
Comment:
   

Attached Files
file #36252:  sortcluster.m added by caliari (726B - application/binary)
file #36253:  testordschur.m added by caliari (1kB - application/binary)
file #33027:  ordschur-corefcn.patch added by svillemot (12kB - text/x-diff)
file #33019:  trsen_less_C.path added by carandraug (9kB - application/octet-stream)
file #33010:  ordschur4.0.patch added by svillemot (12kB - text/x-diff - Updated patch ready for 4.0 release)
file #30463:  ordschur.patch added by svillemot (12kB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by caliari (Updated the item)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by svillemot (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri Feb 5 11:10:44 2016caliariAttached File-=>Added sortcluster.m, #36252
      Attached File-=>Added testordschur.m, #36253
    Wed Feb 18 18:22:20 2015carandraugStatusNeed Info=>Done
      Open/ClosedOpen=>Closed
    Sat Feb 7 20:54:53 2015svillemotAttached File-=>Added ordschur-corefcn.patch, #33027
    Fri Feb 6 18:38:09 2015carandraugAttached File-=>Added trsen_less_C.path, #33019
    Fri Feb 6 14:25:46 2015mtmillerStatusNone=>Need Info
      Assigned toNone=>mtmiller
    Thu Feb 5 22:31:02 2015svillemotAttached File-=>Added ordschur4.0.patch, #33010
    Wed Feb 5 17:23:40 2014svillemotAttached File-=>Added ordschur.patch, #30463

    Back to the top


    Powered by Savane 3.1-cleanup