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

Update CI scripts #211

Merged
merged 2 commits into from
Sep 1, 2020
Merged

Update CI scripts #211

merged 2 commits into from
Sep 1, 2020

Conversation

amontoison
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Aug 31, 2020

Coverage Status

Coverage increased (+0.2%) to 97.466% when pulling 51170de on amontoison:ci into 31a876a on JuliaSmoothOptimizers:master.

@dpo
Copy link
Member

dpo commented Aug 31, 2020

Thanks. Should we at least keep 1.3 since that's the minimum version in Project.toml?

@amontoison
Copy link
Member Author

Yes, good idea.

@dpo
Copy link
Member

dpo commented Sep 1, 2020

Any idea why this errors out only on AMD? https://travis-ci.org/github/JuliaSmoothOptimizers/Krylov.jl/jobs/722902656#L558

@amontoison
Copy link
Member Author

No 🤔 I modified the check_min_norm function a little bit to solve this problem.

@@ -7,7 +7,8 @@ function check_min_norm(A, b, x; λ=0.0)
AI = A
xI = x
end
xmin = AI' * ((AI * AI') \ b)
QR = qr(AI * AI')
Copy link
Member

Choose a reason for hiding this comment

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

Yes, the problem is probably that Julia tries to compute the LU of AA', which is singular. But we only need the QR of A', not of AA'.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right !

@dpo
Copy link
Member

dpo commented Sep 1, 2020

Still failing. How about x = Q * (R' \ b)?

@amontoison
Copy link
Member Author

I forgot to swap R and Rᵀ...
With QR = Aᵀ, AAᵀ = RᵀR and I should use the following code:

QR = qr(AI')
xmin = AI' * (QR.R \ (QR.R' \ b))

@dpo
Copy link
Member

dpo commented Sep 1, 2020

Sure, although multiplying by Q should be less prone to errors than another triangular solve and multiplying by A'.

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #211 into master will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
+ Coverage   97.16%   97.26%   +0.09%     
==========================================
  Files          28       29       +1     
  Lines        2504     2997     +493     
==========================================
+ Hits         2433     2915     +482     
- Misses         71       82      +11     
Impacted Files Coverage Δ
src/cr.jl 57.23% <0.00%> (-2.31%) ⬇️
src/craig.jl 96.72% <0.00%> (-0.37%) ⬇️
src/cg.jl 100.00% <0.00%> (ø)
src/cgs.jl 100.00% <0.00%> (ø)
src/qmr.jl 100.00% <0.00%> (ø)
src/bilq.jl 100.00% <0.00%> (ø)
src/cgls.jl 100.00% <0.00%> (ø)
src/cgne.jl 100.00% <0.00%> (ø)
src/crls.jl 100.00% <0.00%> (ø)
src/crmr.jl 100.00% <0.00%> (ø)
... and 17 more

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 232175b...51170de. Read the comment docs.

@dpo dpo merged commit 45a6cbf into JuliaSmoothOptimizers:master Sep 1, 2020
@dpo
Copy link
Member

dpo commented Sep 1, 2020

🎉

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.

3 participants