[Octave-patch-tracker] [patch #9897] Implement the missing ordqz function.

[Philip Nienhuis]

Follow-up Comment #12, patch #9897 (project octave):

@Markus:
You're right, other than stated in comment the patch probably didn't apply
cleanly last time I applied it.
Looking back I wasn't aware of grisu86's final fix, somehow stumbled on it
(maybe while searching because I needed ordqz) and then applied it maually.

The log of my (quite old, started on June, 14) local-mods-build tree, shows I
have applied 3 csets in the following order:
0 patch_9897_ordqz_function_v3.cset (AFAICS based on original patch, comitted
after fixing a few failed hunks)
0 28478.patch (add missing file, N.B. ordqz.cc itself :-) )
0 28484.patch (fixed the actual bug in original patch + an extra test, see
below).
All attached here for reference.
Together they I think should be equivalent to the first patch + changes in 2nd
patch, rather than the second patch itself.

Now, as to that last test you give in comment #11:

(1) That very test *IS* largely the example on the matlab ordqz page so it
should be dropped or replaced by another one.
(@grisu86: we can't use copyrighted material in Octave, but we can compare
Octave with publicly published Matlab results.)

(2) When run manually it gives (on Windows 10):

>> assert (norm (ECOMP - E2, "Inf"), 0, 1e-8);
error: ASSERT errors for:  assert (norm (ECOMP - E2, "Inf"),0,1e-8)

  Location  |  Observed  |  Expected  |  Reason
     ()       7.2578e-06       0         Abs err 7.2578e-06 exceeds tol 1e-08
by 7e-06

To your relief I think the FAIL is probably either a tolerances issue or a
faulty test; I manually compared the output on that Matlab web page with what
ordqz() in Octave madkes from it. Re-running it again today gives identical
results as Matlab but -as stated before- with sign differences, the latter
already in the output of Octave's qz() function so probably not due to ordqz.

I'd happily try your patch (patch9897_ordqz_v2.patch) (could do somewhere in
between the scenes, or later tonight), but do you think it's still needed with
the above info?


(file #50066, file #50067, file #50068)
    _______________________________________________________

Additional Item Attachment:

File name: patch_9897_ordqz_function_v3.cset Size:4 KB
   
<https://file.savannah.gnu.org/file/patch_9897_ordqz_function_v3.cset?file_id=50066>

File name: 28478.patch                    Size:21 KB
    <https://file.savannah.gnu.org/file/28478.patch?file_id=50067>

File name: 28484.patch                    Size:0 KB
    <https://file.savannah.gnu.org/file/28484.patch?file_id=50068>



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?9897>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/