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.

Sign up for GitHub

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

Jump to bottom

Test unconstrained problems #27

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Test unconstrained problems #27

wants to merge 1 commit into from

Conversation

tmigot
Copy link
Member

@tmigot tmigot commented Jul 23, 2021

No description provided.

@tmigot tmigot linked an issue Jul 23, 2021 that may be closed by this pull request
Test cons and jac for unconstrained problems #26
Open
@codecov
Copy link

codecov bot commented Jul 23, 2021 •
edited
Loading

Codecov Report

Merging #27 (a7ff2ac) into main (d728aa5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head a7ff2ac differs from pull request most recent head 02d5edd. Consider uploading reports for the commit 02d5edd to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main      #27   +/-   ##
=======================================
  Coverage   99.45%   99.45%           
=======================================
  Files          24       24           
  Lines        2367     2406   +39     
=======================================
+ Hits         2354     2393   +39     
  Misses         13       13
Impacted Files Coverage Δ
src/nlp/consistency.jl 99.47% <100.00%> (+0.01%) ⬆️
src/nlp/problems/brownden.jl 100.00% <100.00%> (ø)
src/nls/consistency.jl 99.29% <100.00%> (+<0.01%) ⬆️

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 d728aa5...02d5edd. Read the comment docs.

@github-actions
Copy link
Contributor

Package name latest stable
ADNLPModels.jl
AmplNLReader.jl
CUTEst.jl
CaNNOLeS.jl
DCISolver.jl
JSOSolvers.jl
LLSModels.jl
NLPModelsIpopt.jl
NLPModelsJuMP.jl
PDENLPModels.jl
Percival.jl
QuadraticModels.jl
SolverBenchmark.jl
SolverTools.jl

@tmigot
Copy link
Member Author

tmigot commented Jul 23, 2021

Apparently, some packages are breaking because

jtprod(nlp, nlp.meta.x0, nlp.meta.y0) == zeros(nlp.meta.ncon) # instead of []

which sort of makes sense as it is a product of a ncon x 0 matrix with a 0 x 1 vector, so it returns a ncon x 1 vector.

@dpo @abelsiqueira any opinion? The political way would accept both and just check that jtprod doesn't return an error.

@abelsiqueira
Copy link
Member

... e as it is a product of a ncon x 0 matrix with a 0 x 1 vector, so it returns a ncon x 1 vector.

Correction: nvar instead of ncon.

If I were using jtprod! expecting semi-automatic things to work, I think we should test that jtprod returns a nvar x 1 zero vector.
My rationale:

  • I will create code like
x = rand(nvar)
y = ones(ncon)
Jty = Vector{Float64}(undef, nvar)
jtprod!(nlp, x, v, Jty)
@. dual = gx + Jty

and expect it to work. Possibly even using the 5-arg jac_op, but that's another issue.

  • So we should check all(Jty .== 0).
  • The default jtprod creates Jtv = similar(x), so it should return zeros(nvar) as well.

@github-actions
Copy link
Contributor

Package name latest stable
ADNLPModels.jl
AmplNLReader.jl
CUTEst.jl
CaNNOLeS.jl
DCISolver.jl
JSOSolvers.jl
LLSModels.jl
NLPModelsIpopt.jl
NLPModelsJuMP.jl
PDENLPModels.jl
Percival.jl
QuadraticModels.jl
SolverBenchmark.jl
SolverTools.jl

@tmigot tmigot marked this pull request as draft July 23, 2021 23:13
@github-actions
Copy link
Contributor

Package name latest stable
ADNLPModels.jl
AmplNLReader.jl
CUTEst.jl
CaNNOLeS.jl
DCISolver.jl
JSOSolvers.jl
LLSModels.jl
NLPModelsIpopt.jl
NLPModelsJuMP.jl
PDENLPModels.jl
Percival.jl
QuadraticModels.jl
SolverBenchmark.jl
SolverTools.jl

abelsiqueira
abelsiqueira reviewed Jul 24, 2021
View reviewed changes
Copy link
Member

@abelsiqueira abelsiqueira left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I made some smalls comments.

src/nlp/consistency.jl Outdated
if nlp.meta.ncon == 0
x0, x1 = nlp.meta.x0, ones(nlp.meta.nvar)
@test nlp.meta.nnzj == 0
@test cons ∉ exclude ? cons(nlp, x0) == [] : true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@test cons ∉ exclude ? cons(nlp, x0) == [] : true
@test cons in exclude || cons(nlp, x0) == []

Maybe this is clearer?

src/nlp/problems/brownden.jl Outdated
@@ -129,3 +129,35 @@ function NLPModels.hprod!(
end
return Hv
end

function NLPModels.cons!(nlp, x::AbstractVector{T}, c) where {T}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function NLPModels.cons!(nlp, x::AbstractVector{T}, c) where {T}
function NLPModels.cons!(nlp::BROWNDEN, x::AbstractVector{T}, c) where {T}

@github-actions
Copy link
Contributor

Package name latest stable
ADNLPModels.jl
AmplNLReader.jl
CUTEst.jl
CaNNOLeS.jl
DCISolver.jl
JSOSolvers.jl
LLSModels.jl
NLPModelsIpopt.jl
NLPModelsJuMP.jl
PDENLPModels.jl
Percival.jl
QuadraticModels.jl
SolverBenchmark.jl
SolverTools.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abelsiqueira abelsiqueira abelsiqueira left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test cons and jac for unconstrained problems
2 participants
@tmigot @abelsiqueira