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

Attempt to fix ql #154

Merged
merged 3 commits into from
Nov 27, 2023
Merged

Attempt to fix ql #154

merged 3 commits into from
Nov 27, 2023

Conversation

MasonProtter
Copy link
Member

This change seems to fix the example #153, but it'd be good if there were also tests we could put in to check.

@dlfivefifty
Copy link
Member

Great, thanks for that! I probably changed the interface at some point. It would be great if you could a test.

Btw I forgot that I implemented ∞-QL for such matrices. It probably only works for operators that are asymptotically block Toeplitz, which may not include your original example (it does include bi-infinite discrete Schrödinger operators).

It was very experimental code that hasn't really been used so I'm not sure I'd recommend using it, but if it's something you'd like to explore collaboratively I'd be happy to help. Doing inverse iteration via \ might be better as it uses QR instead of QL.

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2a85133) 79.76% compared to head (a96af3e) 81.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   79.76%   81.50%   +1.74%     
==========================================
  Files          10       10              
  Lines        1379     1379              
==========================================
+ Hits         1100     1124      +24     
+ Misses        279      255      -24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MasonProtter
Copy link
Member Author

MasonProtter commented Nov 21, 2023

It would be great if you could a test.

Yeah, I just wasn't sure what or how to properly test it.

Btw I forgot that I implemented ∞-QL for such matrices. It probably only works for operators that are asymptotically block Toeplitz, which may not include your original example (it does include bi-infinite discrete Schrödinger operators).

It was very experimental code that hasn't really been used so I'm not sure I'd recommend using it, but if it's something you'd like to explore collaboratively I'd be happy to help. Doing inverse iteration via \ might be better as it uses QR instead of QL.

Yep, fair enough!

@dlfivefifty
Copy link
Member

In the first instance you can just do a test that makes sure It doesn't error. Eg something like ql(A-x*I).L[1,1] isa Float64

@dlfivefifty
Copy link
Member

Looks fine, thanks for that!

Let me know if you want to use this more seriously. The philosophy is very much similar to the discussion on Discourse, that is you do the ∞-dimensional tail exactly, but here we construct matrix factorisations rather than inverses. I'd expect it to be more numerically stable; its essentially the difference between QL algorithm and inverse iteration.

@MasonProtter
Copy link
Member Author

Thanks! I think for now, for my puposes, the brute force approach with a truncated matrix is working fine, but I may revisit this soon.

@MasonProtter
Copy link
Member Author

@dlfivefifty do you mind merging this? I have merge rights (through JuliaLinearAlgebra) but it feels a little rude for me to do it since I'm not normally active in this repo.

@dlfivefifty dlfivefifty merged commit f12ffb4 into master Nov 27, 2023
6 checks passed
@dlfivefifty
Copy link
Member

Apologies! I think tests were still running last time I looked

@MasonProtter
Copy link
Member Author

Not a problem :)

@MasonProtter MasonProtter deleted the MasonProtter-patch-1 branch November 27, 2023 15:37
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.

2 participants