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


patch #8307: Add new ordschur function.

Submitted by:  Sébastien Villemot <svillemot>
Submitted on:  Wed 05 Feb 2014 05:23:40 PM UTC  
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)

Wed 18 Feb 2015 06:22:20 PM UTC, comment #7:

Thank you for the patch. I have pushed it, see

Carnë Draug <carandraug>
Project 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>
Project 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 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 MemberIn charge of this item.
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?


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 File(s):

Attached Files
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 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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 18 Feb 2015 06:22:20 PM UTCcarandraugStatusNeed Info=>Done
    Sat 07 Feb 2015 08:54:53 PM UTCsvillemotAttached File-=>Added ordschur-corefcn.patch, #33027
    Fri 06 Feb 2015 06:38:09 PM UTCcarandraugAttached File-=>Added trsen_less_C.path, #33019
    Fri 06 Feb 2015 02:25:46 PM UTCmtmillerStatusNone=>Need Info
      Assigned toNone=>mtmiller
    Thu 05 Feb 2015 10:31:02 PM UTCsvillemotAttached File-=>Added ordschur4.0.patch, #33010
    Wed 05 Feb 2014 05:23:40 PM UTCsvillemotAttached File-=>Added ordschur.patch, #30463

    Back to the top

    Powered by Savane 3.1-cleanup