Don't eval jac_structure if problem has no constraints
#71
Conversation
|
shouldn't you be implementing the empty jacobian in this case? |
|
@tkelman, there's an argument that NLP evaluators for unconstrained models shouldn't need to do anything at all for jacobians |
|
when trying to use a constrained solver? |
|
MPB has no distinction between a constrained and unconstrained solver. I don't see the harm in not asking for jacobian information if we already know that the problem has no constraints. |
|
@abelsiqueira, have you tested if Ipopt invokes the jacobian callbacks when there are no constraints? |
|
No, let me check. |
|
I tested it, it invokes the callback with |
|
@abelsiqueira, that's not an issue since we initialize |
|
It's really IPOPT's job to allocate a vacuous Jacobian (if it really wants to) when a problem is unconstrained. |
|
Not really, the solver API specifies what you're responsible for providing. |
|
It simply makes no sense to ask a user to implement |
|
then you're requiring separate api's for unconstrained vs constrained. from a primarily constrained solver's perspective, it makes perfect sense to only have the more general api. |
|
Nah. |
|
@tkelman, I don't see the harm in accommodating this use case given that it doesn't complicate the code and @abelsiqueira has stepped forward to do the work. If we want to make this more "formally" part of the MPB interface, it would be nice for the unit tests to have a case that covers an unconstrained problem where jac isn't implemented. This also relates to the discussion at JuliaOpt/MathProgBase.jl#87. |
|
Are you going to require every solver to implement this the same way? Seems like you're getting lucky that Ipopt only evaluates this callback in structure mode here, but other constrained solvers may always require the same set of callbacks to be defined and always call them. |
|
I don't know who is calling the Jacobian callback, but perhaps it should not be called for an unconstrained problem. Otherwise, I can redefine the callback when the problem is unconstrained and solve locally. I'm implementing this way because before it was always using the For our models, we're considering prohibitive behaviour on calling functions not in a list of features. |
|
@abelsiqueira, @tkelman's point is that you're now redefining the MPB interface. So let's do it correctly. We need a unit test at https://github.com/JuliaOpt/MathProgBase.jl/blob/master/test/nlp.jl for this behavior and the other open-source solvers (i.e., NLopt) need to be checked and updated as well. |
|
I don't think MPB should be imposing that all constrained solvers have to support this. |
|
@tkelman, it seems like a very small amount of work. Worst case you have a boolean flag that skips the call to |
|
Not if it's built into the way the solver works, several of which we won't be able to change. |
|
@tkelman, what do you mean? This is only at the level at the MPB wrapper which we have full control over. |
|
Aren't MPB wrappers supposed to be low level with as little required complication as possible on top of the solver API's themselves? |
|
@tkelman, even so, replacing |
|
(The translation from the triplet-with-duplicates form to upper-triangular-no-duplicates form in KNITRO is more complicated, in fact.) |
|
If we can establish that Knitro attempts to evaluate the constraints/Jacobian on unconstrained problems, I'm happy to talk about it with its authors. |
|
@abelsiqueira, if you're willing to add the unit tests to MathProgBase as discussed above, I'm okay proceeding with this. |
|
Okay, I'll look into it. |
|
@mlubin , it looks like |
No description provided.