Skip to content

Implement schur_full/schur_vals for Diagonal inputs - #277

Open
lkdvos wants to merge 1 commit into
mainfrom
diagonal-schur
Open

Implement schur_full/schur_vals for Diagonal inputs#277
lkdvos wants to merge 1 commit into
mainfrom
diagonal-schur

Conversation

@lkdvos

@lkdvos lkdvos commented Aug 26, 2026

Copy link
Copy Markdown
Member

Fixes #276: Diagonal input selects DiagonalAlgorithm, which schur never implemented, so schur_full/schur_vals threw a MethodError.

A diagonal matrix is already in Schur form, so T = A, Z = I and vals = diagview(A). Two choices worth flagging:

  • No reordering, unlike the Diagonal path of eig, which sorts. So schur_vals(::Diagonal) != eig_vals(::Diagonal) in ordering; the testsuite's valsc ≈ eig_vals(A) is relaxed accordingly.
  • vals keeps eltype(A) (no complexification), and Z stays Diagonal, as qr/lq keep Q and R diagonal.

Enables the Diagonal schur tests that were commented out as "not supported yet", CPU and CUDA. Note this alone does not fix schur_full(::DiagonalTensorMap): TensorKit omits schur from its default_algorithm forwarding, so selection fails there for every tensor, diagonal or not. That needs a separate PR.

🤖 Generated with Claude Code

Schur reuses eig's algorithm selection, which routes `Diagonal` to
`DiagonalAlgorithm`, but no such implementation existed, so both threw a
`MethodError`. A diagonal matrix is already in Schur form: `T = A`, `Z = I`
and `vals = diagview(A)`, without any reordering.

Fixes #276

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/implementations/schur.jl 92.30% 3 Missing ⚠️
Files with missing lines Coverage Δ
src/implementations/schur.jl 88.65% <92.30%> (+2.45%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Jutho

Jutho commented Aug 27, 2026

Copy link
Copy Markdown
Member

What is the rational for schur_vals of a real diagonal not changing to complex? eig_vals does do this, and schur_vals on the equivalent full matrix would also return complex results.

@lkdvos

lkdvos commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

No rationale... I forgot we had this whole discussion with the eigenvalues as well, I'll change it

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

schur_full/schur_vals select DiagonalAlgorithm for Diagonal but it is not implemented

2 participants