-
Notifications
You must be signed in to change notification settings - Fork 24
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
Migration to IterativeSolvers, bug fixes, increased coverage #65
base: master
Are you sure you want to change the base?
Migration to IterativeSolvers, bug fixes, increased coverage #65
Conversation
@marcusdavidwebb @ajt60gaibb any opinions on this? |
Codecov Report
@@ Coverage Diff @@
## master #65 +/- ##
==========================================
+ Coverage 76.60% 85.90% +9.30%
==========================================
Files 2 1 -1
Lines 483 447 -36
==========================================
+ Hits 370 384 +14
+ Misses 113 63 -50
Continue to review full report at Codecov.
|
We can drop Julia v1.0 support and just require Julia v1.6 if you bump the minor version |
393401b
to
863c512
Compare
I adjusted the Julia compatibility entry, the CI file, and bumped the minor version. Let me know if there's anything else that needs modification. |
There was one test that failed in one out of the six CI runs. I reduced the accuracy requirement of the test to the accuracy requirement of the iterative solver, which should make the test pass. |
I'm interested in the features introduced in this pull request and noticed it has been inactive for nearly two years. Specifically, I'm interested in using Could the maintainers shed light on the status of this PR? Are there unresolved issues or specific reasons preventing its integration? I'm keen to assist, be it resolving merge conflicts or updating the code, to facilitate its inclusion. |
|
Thank you for the response, @dlfivefifty! I'll have a look at this in the next weeks. |
Added adjoint, copy, size, and length for ToeplitzFactorization; added fields n and m to facilitate this. Copied from PR JuliaLinearAlgebra/pull/65 Co-authored-by: Sebastian Ament <[email protected]>
Added adjoint, copy, size, and length for ToeplitzFactorization; added fields n and m to facilitate this. Copied from PR JuliaLinearAlgebra/pull/65 by Sebastian Ament <[email protected]>.
Iterative Solvers
The main goal of this PR is to use the iterative solver routines from IterativeSolvers.jl. To this end, new features include:
lsqr
.gmres
(the current release of ToeplitzMatrices.jl assumes that all symmetric systems are positive definite, and usescg
).New Functionality
Beside this high level goal, the PR also defines several new functions for Toeplitz matrices and factorizations:
inv
forToeplitz
matricesadjoint
,size
, andcopy
forToeplitzFactorization
structuresapply(f, C)
forCirculant
andCirculantFactorization
structuresC
, which appliesf
to the eigenvalues ofC
, in this way defining the matrix functionsabs
,sqrt
, andinv
.Testing Coverage
Further, this PR includes a testing suite with greater coverage.
Bug Fix
In writing the new tests, it became clear that
inv(::TriangularToeplitz)
was broken on Julia 1.7 (and presumable several previous versions too, sincenextpow2
andconv
haven't been inBase
for a while). This PR also fixes the function.