add schur decomposition wrappers #88
Conversation
| Returns | ||
| ------- | ||
|
|
||
| HQ : ndarray |
There was a problem hiding this comment.
I think specifying the exact size of HQ would be good (I gather HQ.shape will be (n, n, p) ?
Similarly, could you say what the size of Tau will be? (this one I couldn't guess)
There was a problem hiding this comment.
HQ.shape is the same shape as input A. Minimum is (max(n, 1), max(n, 1), p) but could be larger.
Tau.shape is (max(1, n-1), p)
There was a problem hiding this comment.
Isn't it worth adding this to the docstring?
If n < min(A.shape[:2]), HQ would still have the same size as A? Would the n: rows and columns of HQ then be uninitialized?
There was a problem hiding this comment.
Yes that's the way many of the SLICOT routines do it. Adding this to the docstring and rebasing once more.
We have a bunch of different docstring styles for the various functions. Opening another issue for that.
There was a problem hiding this comment.
Thank you. Do you agree with this, from #88 (comment):
So should that be "upper diagonal (or Hessenberg) in [:ilo-1]" ?
There was a problem hiding this comment.
Agree. General rule is:
Python-index = Fortran-index - 1Python-slice-start = Fortran-slice-start-1Python-slice-end:Fortran-slice-end
Or put differently:
A(x,y) = A[x-1,y-1]
A(a:b,c:d) = A[a-1:b,c-1:d]
4cf0e1b to
95c3209
Compare
8d0104a to
f480fa4
Compare
|
Please let me know if 1a5d38f looks right to you; it's the same indexing change we discussed for |
|
It looks right, but I have committed another one, also same issue as above with |
|
Edit: oops, I didn't merge on my local branch. You've fixed all this, thanks. I missed that. Ah, I now see what your earlier comment about [ilo-1, ilo-2] was, sorry. Here's how things stand at 4d3a03d;
|
As requested by #86
Please review. Especially the variable names of input and output matrices and the index ranges mentioned in the docstrings.