Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ENH: Add QZ decomposition with tests #185

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

jseabold commented Mar 29, 2012

This adds the generalized Schur decomposition to scipy.linalg. It has been requested on the mailing list a few times. I didn't follow the templating pattern in flapack_user because I have no idea what the syntax for the generic signature in gees__user__routines means.

@pv pv and 1 other commented on an outdated diff Apr 7, 2012

scipy/linalg/decomp_qz.py
+ typb = 'F'
+
+ overwrite_a = overwrite_a or (_datacopied(a1,A))
+ overwrite_b = overwrite_b or (_datacopied(b1,B))
+
+ gges, = get_lapack_funcs(('gges',), (a1,b1))
+
+ if lwork is None or lwork == -1:
+ # get optimal work array size
+ result = gges(lambda x: None, a1, b1, lwork=-1)
+ lwork = result[-2][0].real.astype(np.int)
+
+ if sort is None:
+ sort_t = 0
+ sfunction = lambda x : None
+ else: #TODO, how to specify the external user function?
@pv

pv Apr 7, 2012

Owner

Is this comment outdated?

@jseabold

jseabold Apr 12, 2012

Contributor

yes, removed

@jseabold

jseabold Apr 12, 2012

Contributor

Although, I still don't know how to use the templated external functions as what was already in scipy/linalg/flapack_user.pyf.src. I have no idea what the call signature here means or how to add an optional third parameter as would be needed.

function <prefix>select(<_arg=arg1\,arg2,\0,arg,\2>)

I pinged the dev list, but no one replied. But the hard-coded ones I added work.

@pv pv commented on an outdated diff Apr 7, 2012

scipy/linalg/flapack.pyf.src
+ integer optional,intent(in),depend(jobvsr,n) :: ldvsr=((jobvsr==1)?n:1)
+ <ftype2> intent(out),depend(lwork),dimension(MAX(lwork,1)) :: work
+ integer optional,intent(in),depend(n),check(lwork>=MAX(1,8*n+16)||lwork==-1):: lwork=8*n+16
+ logical optional,intent(hide),depend(n),dimension(n) :: bwork
+ integer intent(out):: info
+ end subroutine <prefix2>gges
+
+ subroutine <prefix2c>gges(jobvsl,jobvsr,sort_t,<prefix2c>select,n,a,lda,b,ldb,sdim,alpha,beta,vsl,ldvsl,vsr,ldvsr,work,lwork,rwork,bwork,info)
+
+ ! For a pair of N-by-N complex nonsymmetric matrices (A,B) computes
+ ! the generalized eigenvalues, the generalized real Schur form (S,T),
+ ! optionally, the left and/or right matrices of Schur vectors (VSL
+ ! and VSR).
+ ! (A,B) = ( (VSL)*S*(VSR)**T, (VSL)*T*(VSR)**H )
+
+ callstatement (*f2py_func)((jobvsl?"V":"N"),(jobvsr?"V":"N"),(sort_t?"S":"N"),cb_<prefix2c>select_in_<prefix2c>gges__user__routines,&n,a,&lda,b,&ldb,&sdim,alpha,beta,vsl,&ldvsl,vsr,&ldvsr,work,&lwork,rwork,bwork,&info,1,1)
@pv

pv Apr 7, 2012

Owner

What are the two parameters after &info ? They're not there in the *gges manpage...

@pv pv and 1 other commented on an outdated diff Apr 7, 2012

scipy/linalg/flapack.pyf.src
@@ -15,6 +16,78 @@ interface
include 'flapack_user.pyf.src'
+ subroutine <prefix2>gges(jobvsl,jobvsr,sort_t,<prefix2>select,n,a,lda,b,ldb,sdim,alphar,alphai,beta,vsl,ldvsl,vsr,ldvsr,work,lwork,bwork,info)
+
+ ! For a pair of N-by-N real nonsymmetric matrices (A,B) computes
+ ! the generalized eigenvalues, the generalized real Schur form (S,T),
+ ! optionally, the left and/or right matrices of Schur vectors (VSL
+ ! and VSR).
+ ! (A,B) = ( (VSL)*S*(VSR)**T, (VSL)*T*(VSR)**T )
+
+ callstatement (*f2py_func)((jobvsl?"V":"N"),(jobvsr?"V":"N"),(sort_t?"S":"N"),cb_<prefix2>select_in_<prefix2>gges__user__routines,&n,a,&lda,b,&ldb,&sdim,alphar,alphai,beta,vsl,&ldvsl,vsr,&ldvsr,work,&lwork,bwork,&info,1,1,1)
@pv

pv Apr 7, 2012

Owner

What are the parameters after &info?

@jseabold

jseabold Apr 12, 2012

Contributor

I have no idea. I followed the wrapper for *gees and it has these extraneous parameters as well AFAICT. I assumed it had something to do with the external function call since in that case select it takes at most two parameters.

@pv

pv Apr 14, 2012

Owner

I'm starting to believe that the *gees declaration is in error: the two arguments are just tacked on to the Fortran routine prototype, so the routine gets called with the wrong signature. Apparently, this does no harm...

I'd suggest you just remove the two last arguments from the callstatement and callprotoargument. Things should still work OK.

Owner

pv commented Apr 7, 2012

This looks good (didn't test it yet, though).

I'd maybe rename the file to _decomp_qz.py to explicitly marked as private, even if the other ones aren't.

Contributor

jseabold commented Apr 12, 2012

Renamed. For some reason I never received a notification on these comments. I just happened to check in.

@rgommers rgommers and 1 other commented on an outdated diff Apr 14, 2012

scipy/linalg/__init__.py
@@ -134,6 +135,7 @@
from decomp_lu import *
from decomp_cholesky import *
from decomp_qr import *
+from decomp_qz import *
@rgommers

rgommers Apr 14, 2012

Owner

Should be from _decomp_qz import *.

@jseabold

jseabold Apr 14, 2012

Contributor

Right, thanks. Fixed.

Owner

rgommers commented Apr 14, 2012

Works fine on OS X 10.6, Python 2.6

@pv pv commented on the diff Apr 14, 2012

scipy/linalg/flapack_user.pyf.src
@@ -6,3 +6,45 @@ python module gees__user__routines
end function <prefix>select
end interface
end python module gees__user__routines
+
+python module zgges__user__routines
@pv

pv Apr 14, 2012

Owner

Here you could use f2py's templating:

The syntax <_arg=arg1,arg2,\0,arg,\2> means the same as <_arg=arg1,arg2,arg1,arg2,arg,arg> -- one of the slashes escapes the comma, and apparently the syntax \0, \1, \2, ... can be used (but IMO shouldn't be because it's confusing) to refer to other values in the expansion.

< ftype> :: <_arg>

expands to

real :: arg1,arg2
double precision :: arg1, arg2
complex :: arg
double complex :: arg

I'd maybe also not name the dummy arguments in such a complicated way --- someone reading this code later on will be confused why there are so many underscores, and whether they are needed by f2py for something magical...

@jseabold

jseabold May 3, 2012

Contributor

Sorry forgot to look into this. Thanks for the pointer. This is still very murky to me. I still don't see why arg, \2 expands to arg2, arg, arg and why arg, arg should be assumed to be a complex number.

Owner

rgommers commented Apr 14, 2012

Could you also add .. versionadded:: 0.11.0 to the Notes section of the docstring, and mention this in the release notes?

albop commented on da19fbf May 3, 2012

On my computer Ubuntu 12.04 64, the test test_qz_double fails.
I get the message: error: (lwork>=MAX(1,8*n+16)||lwork==-1) failed for 6th keyword lwork: dgges:lwork=52
Looking at the code, I see that the parameter lwork computed line 156 is indeed incorrect for double types. This can be reproduced by running :

from scipy.linalg.lapack import get_lapack_funcs
n = 5
#A = random([n,n]).astype(complex64)
#B = random([n,n]).astype(complex64)
A = random([n,n]).astype(float64)
B = random([n,n]).astype(float64)
qqz, = get_lapack_funcs(('gges',), (A, B) )
lwork = qqz(lambda x: None, A,B, lwork=-1)[-2][0].astype(int)
print lwork

I have also checked that a direct fortran call with lwork seems to work ( qqz(lambda x: None, A,B) gives the expected result) so I wonder why we need to call the fortran code two times in case the user hasn't supplied lwork to the python function.

Owner

jseabold replied May 3, 2012

Thanks for the report. Unfortunately, I can't reproduce this. What version of lapack are you using?

from scipy.linalg.lapack import get_lapack_funcs
import numpy as np

n = 5
A = np.random.random([n,n]).astype(np.float64)
B = np.random.random([n,n]).astype(np.float64)
qqz, = get_lapack_funcs(('gges',), (A, B))
lwork = qqz(lambda x: None, A,B, lwork=-1)[-2][0].astype(int)
print lwork

lwork = qqz(lambda x: None, A,B)[-2][0].astype(int)
print lwork

Both give 61, though the former call just calculates this value and returns. I have ATLAS 3.9.52 with LAPACK 3.3.1.

albop replied May 3, 2012

Owner

jseabold replied May 3, 2012

Honestly, I have no idea why there are problems. From my end, everything looks ok in the Python code and the Fortran wrappers. I just tried again on another machine that has Ubuntu 10.04 and ATLAS 3.6.0 and LAPACK 3.2.1 from the Ubuntu respositories (my other ones are compiled from source). Everything is fine there even though it gives a different optimal lwork. It is still consistent and works.

I pinged the scipy-dev list to see if anyone else has any insight.

Owner

jseabold replied May 3, 2012

Ok, I found the problem. Could you confirm that the recent changes fix things for you?

albop replied May 3, 2012

Apparently the errors are gone. Good work !

@pv pv added a commit that referenced this pull request May 19, 2012

@pv pv Merge pull request #185 from jseabold:add-qz-decomp
  ENH: linalg: use f2py templating in gges SELCTG generation
  ENH: Keep the larger documented default for lwork
  BUG: Fix minimum lwork value to match source and not docs
  REF: Remove extra arguments from wrapper
  DOC: Add release related documentation
  ENH: Fix import after rename
  STY: Rename decomp_qz.py -> _decomp_qz.py
  STY: Remove outdated comment
  ENH: Add QZ decomposition with tests
63291d1
Owner

pv commented May 19, 2012

Merged in 63291d1
Thanks a lot!

@pv pv closed this May 19, 2012

@jseabold jseabold deleted the jseabold:add-qz-decomp branch Apr 21, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment