search

JuliaLinearAlgebra / IterativeSolvers.jl

Iterative algorithms for solving linear systems, eigensystems, and singular value problems
MIT License
394 stars 106 forks source link

Add COCG method for complex symmetric linear systems #289

Open wsshin opened 3 years ago

wsshin commented 3 years ago

This PR adds the conjugate orthogonal conjugate gradient (COCG) method for complex symmetric matrices A (i.e., transpose(A) == A rather than adjoint(A) == A). The PR implements cocg and cocg! functions (corresponding to cg and cg! for Hermitian matrices), supporting functions and types, and basic unit tests.

This PR is related with #288.

wsshin commented 3 years ago

I haven't added any documentation pages for COCG, as IterativeSolvers.jl seems to use Documenter.jl, which I haven't used before. Please let me know if there are some basic guidelines as to how to create documentation pages using Documenter.jl.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 508517998

Totals Coverage Status
Change from base Build 444205317: -0.3%
Covered Lines: 1873
Relevant Lines: 1927
💛 - Coveralls
coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 652039845

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

Totals Coverage Status
Change from base Build 444205317: 0.3%
Covered Lines: 1939
Relevant Lines: 1982
💛 - Coveralls
haampie commented 3 years ago

Hi @wsshin, thanks for your contribution!

I think another way to do this is to dispatch to a different dot-function inside of CG based on a tag.

For instance:

struct InnerProduct end
struct UnconjugatedProduct end

_norm(::InnerProduct, xs) = norm(xs)
_dot(::InnerProduct, xs, ys) = dot(xs, ys)

_norm(::UnconjugatedProduct, xs) = sqrt(sum(x^2 for x in xs))
_dot(::UnconjugatedProduct, xs, ys) = sum(prod, zip(xs, ys))

and then add this tag InnerProduct / NoInnerProduct to the CG iterable type. That saves some duplication.

Is there a name for this non-conjugate innerproduct-like operation sum(prod, zip(xs, ys))?

wsshin commented 3 years ago

@haampie, thanks for the suggestion. Please let me know if the new commit fulfills your request.

wsshin commented 3 years ago

Not sure why codecov generated a failure. Tried to remove it by adding more unit tests, but it didn't work. Please advise.

wsshin commented 3 years ago

@haampie, any thoughts on the new commit in this PR? Hope to finish this PR soon and move on to the implementation of COCR as mentioned in #288.

haampie commented 3 years ago

Hi @wsshin , i'll review in the weekend if that's ok with you.

wsshin commented 3 years ago

@haampie, please let me know if you don't agree with my opinions; I can accommodate yours.

wsshin commented 3 years ago

I think I have addressed all the concerns raised so far. If there is anything else, please let me know.

wsshin commented 3 years ago

Can you also list this method in the docs? https://julialinearalgebra.github.io/IterativeSolvers.jl/dev/linear_systems/cg/ maybe as part of this page?

So this is the part I was curious about. I haven't created docs for Julia packages before. Is there a quick guide for creating docs?

haampie commented 3 years ago

You can do

$ cd path/to/IterativeSolvers/docs
$ julia
julia> using Revise

(@v1.5) pkg> activate .
 Activating environment at `~/.julia/dev/IterativeSolvers/docs/Project.toml`

(docs) pkg> instantiate

julia> include("make.jl") # repeatedly

and edit docs/* and open build/index.html in your browser

stevengj commented 3 years ago

@wsshin, see https://juliadocs.github.io/Documenter.jl/stable/ … you might add a note to https://github.com/JuliaLinearAlgebra/IterativeSolvers.jl/blob/master/docs/src/linear_systems/cg.md and https://github.com/JuliaLinearAlgebra/IterativeSolvers.jl/blob/master/docs/src/index.md

(The above commands are only only needed to build the doc pages locally; they will be built automatically when a PR is merged.)

wsshin commented 2 years ago

@haampie, it took a while for me to add the documentation for COCG. Hopefully the latest commits are good to be merged. Note that CI tests are failing for Julia 1, but the failures are from gmres, not from cg or cocg.