change functional constraint functions #116
Conversation
|
@BaptisteCbl hi there, is this PR ready to merge? the case being I'll have a look. |
|
Hello, this PR only addresses the constraint revise, but is ready to merge in this matter per se. |
| julia> constraint!(ocp, :state, rg=2:3, lb=[ 0, 0 ], ub=[ 1, 2 ]) | ||
| julia> constraint!(ocp, :initial, rg=1:2:5, lb=[ 0, 0, 0 ], ub=[ 1, 2, 1 ]) | ||
| julia> constraint!(ocp, :variable, rg=1:2, lb=[ 0, 0 ], ub=[ 1, 2 ]) | ||
| ``` | ||
| """ | ||
| function constraint!(ocp::OptimalControlModel{<: TimeDependence, <: VariableDependence}, type::Symbol; | ||
| rg::Union{RangeConstraint,Nothing}=nothing, f::Union{Function,Nothing}=nothing, | ||
| val::Union{ctVector,Nothing}=nothing, lb::Union{ctVector,Nothing}=nothing, ub::Union{ctVector,Nothing}=nothing, | ||
| rg::Union{OrdinalRange{<:Integer},Integer,Nothing}=nothing, f::Union{Function,Nothing}=nothing, lb::Union{ctVector,Nothing}=nothing, ub::Union{ctVector,Nothing}=nothing, |
There was a problem hiding this comment.
@BaptisteCbl 👍🏽 thanks, that was the point
| (typeof(rg) <: Int) && (rg = Index(rg)) | ||
|
|
||
| @match (rg,f,lb,ub) begin | ||
| (::Nothing,::Nothing,::ctVector,::ctVector) => return constraint!(ocp, type, lb, ub, label) # |
There was a problem hiding this comment.
@BaptisteCbl at this point, I see that you reduce the computation of the new constraint! functions with updated named arguments to three cases/subfunctions:
- linear, full range (no range give implies full range)
- linear, given range
- nonlinear (function)
Wouldn't it be more robust, particularly with less code duplication (e.g., check on arguments), to do the converse:
- fully code the new version with named args
- implement the other calls of
constraint!(namely the three cases mentioned above) by calling this unique new function?
| @@ -4,29 +4,30 @@ | |||
| using BenchmarkTools | |||
There was a problem hiding this comment.
@BaptisteCbl just for the record: can you please recall me what these bench folder and bench_nlp_constraints are for? (did not remember this stuff.)
There was a problem hiding this comment.
The bench folder contains programs to generate a markdown file with the performance of the Julia file. Apart from the examples that use CTBase, it is independent of CTBase. I don't think it was meant to stay here. But it was actually developed by @ocots, he may have a better answer.
There was a problem hiding this comment.
Hi @BaptisteCbl, many thanks for the work 🙏🏽 Main comment here. Please tell me what think.
|
@BaptisteCbl hi again, please tell me if you have touched the request below (main comment on the
|
|
Hi, I haven't touched this request either but I indeed think that it would be better with your suggestion. |
@BaptisteCbl Thanks for the confirmation. Please tell me if you want to do it (I'll do it otherwise 🥹) |
|
I won't be able to do it today or this weekend, but if you want I can do it on Monday. |
No hurry, next week is fine 👍🏽. Keep in touch. |
|
@BaptisteCbl having some time this Friday to work on this with you. keep me informed. |
|
Hi, I looked at the constraints! functions and tried to do as you said but I have difficulty having functions that do not overwrite themselves when we keep the same options. In fact, since we can have declaration with named arguments |
|
Hi @BaptisteCbl ; available for a quick catch up this Friday (Oct. 20)? |
|
Hello, yes I am available in the morning before 12h. |
👍🏽 I’ll be on zoom at 10:30 if you can |
|
Followed by [#123] |
Hi,
This is the pr related to the issue #113 (constraint revise).
I'll do an other pr or commits in this one for the time function revise #115.