Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace {s,d}laqrb with {s,d}lahqr #19

Merged
merged 8 commits into from
Sep 17, 2015
Merged

Conversation

thrasibule
Copy link
Contributor

Issue #3 was partially solved with pull request #2, by bringing back an older version of dlahqr.
However this only worked with reference blas, but with atlas or openblas, the issue persists as evidenced by #7, #16 and #17. This is another attempt at fixing it using a strategy suggested here by Marco Caliari.

This is an attempt to close #7 ,close #16 and close #17.

@sylvestre
Copy link
Contributor

Impressive, could you also update the CHANGES to describe these changes? Thanks

@thrasibule
Copy link
Contributor Author

added a description in the CHANGES file. All the credit for this idea goes to Marco Caliari, I just put the pull request together...

sylvestre added a commit that referenced this pull request Sep 17, 2015
* reverts using {d,s}lahqr from lapack 2
* use dlahqr from lapack 3 instead of dlaqrb (credit to Marco Caliari)
@sylvestre sylvestre merged commit 34882f4 into opencollab:master Sep 17, 2015
@sylvestre sylvestre changed the title Dlaqrb replace {s,d}laqrb with {s,d}lahqr Oct 12, 2015
@caliarim
Copy link
Contributor

caliarim commented Jan 8, 2016

Dear maintainers,

thanks for including my proposal in 3.3.0. In the old dlaqrb.f there was this piece of code

 if( wantt ) then
     i1 = 1
     i2 = n
     do 8 i=1,i2-2
        h(i1+i+1,i) = zero

8 continue
else
do 9 i=1, ihi-ilo-1
h(ilo+i+1,ilo+i-1) = zero
9 continue
end if

which is not present in dlahqr. Since dlaqrb.f was always called with wantt = .true., only the first branch is used. I don't know whether it is really necessary to zero out h. But it can be achived in dneigh.f by

do 5 j = 1, n-2
bounds(j) = zero
workl(j+2+n*(j-1)) = zero
5 continue
bounds(n-1) = zero
bounds(n) = one
call dlahqr(.true., .true., n, 1, n, workl, n, ritzr, ritzi, 1,
& 1, bounds, 1, ierr)

@thrasibule thrasibule deleted the dlaqrb branch January 20, 2016 22:48
@thrasibule
Copy link
Contributor Author

@caliarim. If I understand the code correclty, this sets the entries on the second diagonal to be 0. h(i+2,i)=0 for 1<=i<=n-2. don't think this is needed. With the line:

call dlacpy ('All', n, n, h, ldh, workl, n)

h is copied into workl which is an upper hessenberg matrix so all entries below the subdiagonal should be 0. What do you think?

@caliarim
Copy link
Contributor

Look, for instance, at line 552 of dnaupd2. h(3,1) is used as storage.

@thrasibule
Copy link
Contributor Author

Well spotted! However, in dlahqr code, I see:

*     ==== clear out the trash ====
      DO 10 J = ILO, IHI - 3
         H( J+2, J ) = ZERO
         H( J+3, J ) = ZERO
   10 CONTINUE

So seems that it's already taken care of. Do you agree?

@caliarim
Copy link
Contributor

Ah, right! This piece of code is not in dlahqr 2.0, and therefore it was inserted into dlaqrb. I agree with you that it is not necessary to zero out workl before calling dlahqr. To be 100% sure, I initialized workl(j+2+n_(j-1)) with (-1.0d0)__j_1000.d0 and arpack worked without problems.

@thrasibule
Copy link
Contributor Author

Cool! I'm glad this is sorted out.

pv added a commit to pv/scipy-work that referenced this pull request Feb 28, 2016
Using LAPACK 2.0 versions of *lahqr routines was a workaround for an
ARPACK bug (its *laqrb routines having not been updated for LAPACK 3.x).
This is fixed in ARPACK-NG >= 3.2.0

See discussion at
http://forge.scilab.org/index.php/p/arpack-ng/issues/1315
opencollab/arpack-ng#2
opencollab/arpack-ng#19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants