NLPModels.jl icon indicating copy to clipboard operation
NLPModels.jl copied to clipboard

Documentation: inconsistency in whether constraints are zero

Open timholy opened this issue 1 year ago • 11 comments

Consider HS62, for which the constraint is

x[1] + x[2] + x[3] - 1 == 0

and the initial value is x0 = [0.7, 0.2, 0.1] which sums to 1. Thus you expect the initial constraint value to be zero. However:

julia> using NLPModels, NLPModelsJuMP, OptimizationProblems.PureJuMP

julia> nlp = MathOptNLPModel(hs62());

julia> cons(nlp, nlp.meta.x0)
1-element Vector{Float64}:
 0.9999999999999999

My suspicion is that the bug is in this package, but I'm unsure. It seems likely that it's neglecting to incorporate the constant offset in the JuMP model?

timholy avatar Apr 20 '24 15:04 timholy

Hi @timholy! I should document it but we store the linear and quadratic equality constraints as Ax = b and x'Qx + dx = c in NLPModelsJuMP.jl. So all constant terms in these constraints are moved in the right-hand side.

amontoison avatar Apr 20 '24 15:04 amontoison

But doesn't the fundamental problem represented by NLPModels say that it solves c(x) = 0? And that cons is the way you evaluate the constraints? https://jso.dev/NLPModels.jl/stable/

Is there the equivalent of lcons (left constraint) and rcons (right constraint)? In which case one could say that the fundamental problem to be solved is lcons(nlp, x) == rcons(nlp, x).

timholy avatar Apr 20 '24 15:04 timholy

It solves c(x) = 0 but internally we can store it as Ax = b when the constraints are linear.

Capture d’écran du 2024-04-20 11-44-57

cc @tmigot

amontoison avatar Apr 20 '24 18:04 amontoison

Is there the equivalent of lcons (left constraint) and rcons (right constraint)? In which case one could say that the fundamental problem to be solved is lcons(nlp, x) == rcons(nlp, x).

Yes, you have this information in nlp.meta. https://jso.dev/NLPModels.jl/dev/reference/#NLPModels.NLPModelMeta You can use this code:

con(nlp, x)[nlp.meta.jfix] ≈ nlp.meta.lcon[nlp.meta.jfix]

or

con(nlp, x)[nlp.meta.jfix] ≈ nlp.meta.ucon[nlp.meta.jfix]

nlp.meta.lcon and nlp.meta.ucon and the lower and upper bounds of the constraints. nlp.meta.jfix is the index of the equality constraints.

amontoison avatar Apr 20 '24 19:04 amontoison

Sounds fine, I can understand why internally you might do it this way. I'm pointing out, though, that this change appears to introduce a major inconsistency in your API. Here's the docstring for cons:

help?> cons
search: cons const cons! consjac conscale cons_nln cons_lin cons_nln! cons_lin! cons_coord cons_coord! isconst unconstrained condskeel objcons objcons! MathConstants

  c = cons(nlp, x)

  Evaluate c(x), the constraints at x.

and then the docs for NLPModels have this: image

What I'm pointing out is that cons doesn't compute what it claims to compute. It's fine if you represent things differently internally, but cons should return the actual constraint value (and it should be zero at the solution, which isn't true currently).

It would be a little weird if the docstring were "cons computes the constraint value, except if there is a constant offset, in which case it's everything except the constant." Either cons should just be deleted or it needs to return the actual constraint value.

timholy avatar Apr 20 '24 22:04 timholy

I agree with you. I think we only have the issue in the package because of how I parse the JuMP model here. We can update cons to return the actual constraint value.

amontoison avatar Apr 20 '24 22:04 amontoison

There seems to be an inconsistency in the docs. Below the equation you quote, we state that the constraints are $c_L \leq c(x) \leq c_U$, and that the $i$-th components of $c_L$ and $c_U$ are equal for equality constraints. The constraint is $c_i(x)$. The common value of $c_L$ and $c_U$ is the “right”-hand side; it is not part of the constraint.

dpo avatar Apr 20 '24 22:04 dpo

Sorry, just circling back to this. @dpo I guess your point is that given the desirability of (notationally) unifying equalities and inequalities, the better approach is to drop the guarantee of $c_i(x) = 0$ for all equality constraints. That seems reasonable. I'd certainly be fine with using the suggestion in https://github.com/JuliaSmoothOptimizers/NLPModels.jl/issues/460.

I've edited the title of this issue accordingly. Perhaps someone with appropriate privileges should label it a documentation issue and transfer it to NLPModels instead?

timholy avatar Apr 26 '24 08:04 timholy

@timholy If you prefer the right-hand side of your equality constraints to be zero, it’s enough to build it into the constraint and set the right-hand side to zero explicitly when you define the model. In other words, the safe thing to do is to always evaluate $c(x) - c_L$ when testing whether an equality constraint is nearly satisfied or not.

dpo avatar Apr 26 '24 16:04 dpo

Right. To be clear, the way this came up was using OptimizationProblems.jl as a repository of test problems for an external optimizer, and then being surprised that the solution wasn't as stated in the documentation. It's really just a documentation issue, but one that's a significant "gotcha" for external consumers of your interface.

timholy avatar Apr 28 '24 07:04 timholy

Thanks for bringing this to our attention!

dpo avatar Apr 29 '24 17:04 dpo