Provide the user_data to the callbacks#514
Provide the user_data to the callbacks#514amontoison wants to merge 1 commit intojump-dev:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #514 +/- ##
===========================================
- Coverage 100.00% 99.48% -0.52%
===========================================
Files 5 5
Lines 1154 1170 +16
===========================================
+ Hits 1154 1164 +10
- Misses 0 6 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm not really in favour of this. What's the problem with a closure? |
|
Do you have a worked example of code that would be improved with this? |
|
Can't this just be achieved with: prob = Ipopt.CreateIpoptProblem(
n,
x_L,
x_U,
m,
g_L,
g_U,
nnzJ,
nnzH,
x -> eval_f(user_data, x),
(args...) -> eval_g(user_data, args...),
(args...) -> eval_grad_f(user_data, args...),
(args...) -> eval_jac_g(user_data, args...),
(args...) -> eval_h(user_data, args...),
)I don't really want to make changes to the C API, and adding multiple ways of doing something has a long-term maintenance cost. |
|
You can convince me of the need for this at JuMP-dev |
|
Conclusion is that we need an example of why we need this. It might have something to do with Enzyme. |
| @@ -148,19 +178,36 @@ function _Intermediate_CB( | |||
| try | |||
| return reenable_sigint() do | |||
| prob = unsafe_pointer_to_objref(user_data)::IpoptProblem | |||
There was a problem hiding this comment.
Don't we need IpoptProblem{M} here to avoid type-instabilities ?
@odow
When I created the C / Julia interface for
Uno, I checked a few times the interfaces of other nonlinear solvers (Ipopt,KNITRO,GALAHAD) and I noticed that inIpopt.jlwe never provide a way to passuser_data.This means that
model::Optimizeris passed as a closure, and the same applies tomodel::AbstractNLPModelinNLPModelsIpopt.jl.In
UnoSolver.jl, I added an option to passuser_data/user_modelwhen creating the internal solver model (Ipopt.IpoptProblemin this repository).If
user_dataisnothing, we keep the current signature; otherwise, we passuser_data/user_modelas the first argument to all Julia callbacks.This is what we expect in the signatures of the NLP API in
MathOptInterface.jlorNLPModels.jl.I also think it is a clean way to pass a list of parameters to an objective or constraints.
For example, if we have a complex physical problem, we would like to store them in a Julia structure and pass this structure as an argument to the Julia functions.
I find it neater than using closures.
The current PR is non-breaking because, by default,
user_data/user_modelisnothing.I would like to have your point of view (as well as that of other users) before I add unit tests for coverage.
Ipopt.jl: https://github.com/jump-dev/Ipopt.jl/blob/master/ext/IpoptMathOptInterfaceExt/MOI_wrapper.jl#L1242-L1264UnoSolver.jl: https://github.com/cvanaret/Uno/blob/main/interfaces/Julia/ext/UnoSolverMathOptInterfaceExt/MOI_wrapper.jl#L1294-L1301