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

SYMMLQ improvements #138

Merged
merged 1 commit into from
Dec 9, 2019
Merged

Conversation

amontoison
Copy link
Member

I modified SYMMLQ to be able to compute x_lq when x_cg doesn't exist.
ζ = ζbar * c is equal to Inf if ζbar = Inf / γbar = 0.
The η variable has been introduce to solve this problem.

src/symmlq.jl Outdated Show resolved Hide resolved
src/symmlq.jl Outdated

rNorm = sqrt(γ * γ * ζ * ζ + ϵold * ϵold * ζold * ζold)
rcgNorm = abs(rcgNorm*s*cold/c)
rcgNorm = β * abs(s * ζ - c * ζbar)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide more information on these two changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check if gammabar = 0 ?

@dpo dpo changed the title SYMMLQ improvments SYMMLQ improvements Oct 10, 2019
@amontoison amontoison force-pushed the symmlq_singular branch 2 times, most recently from a3a5d63 to 7a44db5 Compare November 18, 2019 20:07
@amontoison
Copy link
Member Author

amontoison commented Nov 18, 2019

I rebase this pull request.

Contrary to BiLQ or USYMLQ, I don't check that γbar ≠ 0 when I compute ζbar. We need it to compute CG residual norms and error norms bounds that are returned in a SymmlqStats at the end. Nevertheless I still check that γbar ≠ 0 iN solved_cg.

I also add test problems where CG, BiCG and USYMCG don't exist at the first iteration and verify that our implementations / LQ factorizations handle it even if T_k is singular (γbar = 0).

@coveralls
Copy link

coveralls commented Nov 18, 2019

Coverage Status

Coverage increased (+0.07%) to 96.277% when pulling cb6a961 on amontoison:symmlq_singular into f5419ff on JuliaSmoothOptimizers:master.

@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

Merging #138 into master will decrease coverage by 0.11%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
- Coverage   97.16%   97.05%   -0.12%     
==========================================
  Files          26       26              
  Lines        2155     2176      +21     
==========================================
+ Hits         2094     2112      +18     
- Misses         61       64       +3
Impacted Files Coverage Δ
src/Krylov.jl 100% <ø> (ø) ⬆️
src/symmlq.jl 96.95% <92.72%> (-2.37%) ⬇️
src/krylov_utils.jl 95% <95%> (-5%) ⬇️
src/krylov_aux.jl 96.25% <0%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5419ff...cb6a961. Read the comment docs.

@dpo dpo mentioned this pull request Nov 20, 2019
@dpo
Copy link
Member

dpo commented Nov 20, 2019

I'm not thrilled about having Infs in the code when gammabar = 0. I think we should test like in the other solvers?!

@amontoison
Copy link
Member Author

amontoison commented Nov 20, 2019

My problem is what should we do with rcgNorm and errorscg if gammabar = 0?
A solution could be to not update them, but the drawback is that we loose the link between iter and rcgNorm[iter]. We don't know anymore at which iteration errors norm or residuals norm have been computed.

@dpo
Copy link
Member

dpo commented Nov 21, 2019

How do you do it in BiLQ? We could insert a missing value for the CG residual and error, which would avoid using Infs and NaNs.

@amontoison
Copy link
Member Author

In BiLQ and USYMLQ, residual norms are computed only if CG point exists (δbarₖ ≠ 0). With those methods, a SimpleStats is returned at the end. So it's not a problem if rcgNorms are not computed because we don't need to store them.
With SYMMLQ, more elaborated statistics are returned. They include residuals and errors norms of CG points. Using missing value if a CG point doesn't exist is a good idea.

@dpo
Copy link
Member

dpo commented Nov 22, 2019

In BiLQ and USYMLQ, residual norms are computed only if CG point exists (δbarₖ ≠ 0). With those methods, a SimpleStats is returned at the end. So it's not a problem if rcgNorms are not computed because we don't need to store them.

BiLQ and USYMLQ could (should) also return CG residuals, even if they don't form the CG iterate until the end.

@amontoison
Copy link
Member Author

amontoison commented Nov 22, 2019

I think we could change SymmlqStats to LQStats in an other pull request for all LQ methods (LSLQ, LNLQ, SYMMLQ, BiLQ, USYMLQ).

@dpo
Copy link
Member

dpo commented Nov 22, 2019

sounds good.

@dpo dpo merged commit 54456f1 into JuliaSmoothOptimizers:master Dec 9, 2019
@dpo
Copy link
Member

dpo commented Dec 9, 2019

Thank you.

@amontoison amontoison mentioned this pull request Dec 9, 2019
@amontoison amontoison deleted the symmlq_singular branch August 4, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants