Revise constraint!

[jbcaillau]

@ocots Following this commit:

• OK to add getters for such information as (pure) control constraint dimensions, etc.
• not nice to call nlp_constraints to do it! (does a lot of other things)
• I suggest to (i) remove the notion of path constraint (sum control + state + mixed, not used, as far as I know), (ii) stick more strictly to the classification we introduced:
  • :initial, :final, :boundary, :state, :control, :mixed, :variable
  • either constrained using range (what you - relevantly - called box) or functions

I am having a look at it.

EDIT: path constraint used by @PierreMartinon in CTDirect.jl. keeping it.

[ocots]

I have added set_dim_constraints after Pierre added the computations of the dimensions inside constraints!. It was not my best idea but I tried to make the getters to get dimensions more consistent. Maybe we could compute the dimensions directly inside the getters.

[jbcaillau]

@ocots

It was not my best idea

💔

Maybe we could compute the dimensions directly inside the getters.

❤️

[ocots]

@PierreMartinon did an ugly thing with the computations of the dimensions inside the constraints! function.

[PierreMartinon]

@ocots Following this commit:

* OK to add getters for such information as (pure) control constraint dimensions, _etc._

Yeah

* not nice to call `nlp_constraints` to do it! (does a lot of other things)

Agreed, I did it the quick and dirty way. Definitely room for improvement.

* I suggest to (i) remove the notion of _path constraint_ (sum control + state + mixed, not used, as far as I know)

I use it in CTDirect but you can remove it for clarity, no problem.

(ii) stick more strictly to the classification we introduced:
* :initial, :final, :boundary, :state, :control, :mixed, :variable

Not sure what you mean but these 6 are fine for me yes.

  * either constrained using range (what you - relevantly - called _box_) or functions

I wonder if there may be some misunderstanding here @ocots @jbcaillau

Ipopt calls 'box' constraints the very simple form LB <= X <= UB, ie where the constraint function is the variable itself, f:(x,u,v)->(x,u,v) in OCP notations. Everything else goes into the nonlinear constraints LB <= F(X) <= UB. Do we have the same 2 categories in the OCP ?

What confuses me is, when you say 'range', do you refer to the indices (argument rg of constraints) or the box (lb,ub) ?

[PierreMartinon]

@PierreMartinon did an ugly thing with the computations of the dimensions inside the constraints! function.

nlp_constraints actually no ?

[ocots]

@PierreMartinon did an ugly thing with the computations of the dimensions inside the constraints! function.

nlp_constraints actually no ?

Right.

[jbcaillau]

gouzi @PierreMartinon the blame was on you, not on poor cotsee 🥹... I'm on it.

[jbcaillau]

@PierreMartinon

• I suggest to (i) remove the notion of path constraint (sum control + state + mixed, not used, as far as I know)
  I use it in CTDirect but you can remove it for clarity, no problem.

✅ kept

• either constrained using range (what you - relevantly - called box) or functions

I wonder if there may be some misunderstanding here @ocots @jbcaillau

Ipopt calls 'box' constraints the very simple form LB <= X <= UB, ie where the constraint function is the variable itself, f:(x,u,v)->(x,u,v) in OCP notations. Everything else goes into the nonlinear constraints LB <= F(X) <= UB. Do we have the same 2 categories in the OCP ?

What confuses me is, when you say 'range', do you refer to the indices (argument rg of constraints) or the box (lb,ub) ?

Sorry, should've been more explicit:

• constraints are either linear (= ranges = projections) or general / nonlinear (= functions). (Actually, considering that only ranges are for sure linear is a safe option 🙂)
• in both cases, linear / nonlinear, there are boxes (= lower and upper bounds), that's why I don't like very much the _box suffix

Currently updating all this, I can provide aliases in CTDirect.jl if you'd rather keep the _box denomination.

[jbcaillau]

@PierreMartinon actually, you already have everything you need just retrieving the size of the bounds returned (together with the functions and ranges) by nlp_constraint:

return (ξl, ξ, ξu), (ηl, η, ηu), (ψl, ψ, ψu), (ϕl, ϕ, ϕu), (θl, θ, θu), (ul, uind, uu), (xl, xind, xu), (vl, vind, vu)
dim_state_con = length(ηl) # same as ηf (this is now @assert-ed)

And it would not be very nice (though not crazily costly) to provide individual getters for these informations (one dictionnary run for each info) that can be retrieved once and for all.

@ocots I am leveraging this to remove pushes in nlp_constraint and allocate things once and for all. Will solve #114.

NB. Another step would be to allow in place computations form within @def.

[jbcaillau]

@ocots replacing push by slices affectations, including length-one ones 🥲

julia> x[1:1] = 1
ERROR: ArgumentError: indexed assignment with a single value to possibly many locations is not supported; perhaps use broadcasting `.=` instead? [...]

julia> x[1:1] .= 1
1-element view(::Vector{Int64}, 1:1) with eltype Int64:
 1

julia> x[1:2] .= [1, 2]
2-element view(::Vector{Int64}, 1:2) with eltype Int64:
 1
 2

[jbcaillau]

@ocots please review and merge #165

@PierreMartinon closing and going to control-toolbox/CTDirect.jl#120

[PierreMartinon]

@PierreMartinon actually, you already have everything you need just retrieving the size of the bounds returned (together with the functions and ranges) by nlp_constraint:

return (ξl, ξ, ξu), (ηl, η, ηu), (ψl, ψ, ψu), (ϕl, ϕ, ϕu), (θl, θ, θu), (ul, uind, uu), (xl, xind, xu), (vl, vind, vu)
dim_state_con = length(ηl) # same as ηf (this is now @assert-ed)

Of course, that's what I did before. The point was to add getters for these dimensions in CTBase ;-)