Add objective sense into the front-end
See http://www.juliaopt.org/MathOptInterface.jl/dev/apireference/#MathOptInterface.ObjectiveSense
The default should be min_sense and you might even want to only implement that in the first iteration, but you want to do the API for this correctly from the beginning.
certainly MIN_SENSE and MAX_SENSE are necessary. The last one there is FEASIBILITY_SENSE is for when you are using an optimizer to solve a feasibility problem, but not sure if it is only used in problem transformation or not. The feasibility approach is to solve a system of equations by using the nonlinear optimizer/etc. to find a feasible point for the constraints where you don't care about the objective value.
Regardless, it means that you wouldn't want to design as a min vs max toggle in case feasibility or others should come later.
If you don't liek the word "sense" then objective_goal is another one. e.g. https://www.artelys.com/docs/knitro/3_referenceManual/callableLibraryAPI.html#basic-problem-construction
Actually, looks like ipopt MOI wrapper uses FEASIBILITY_SENSE directly.
For example, set the objective to be 0
https://github.com/JuliaOpt/Ipopt.jl/blob/master/src/MOI_wrapper.jl#L573-L584
Just a reminder on this one for API design
If you haven't already, and I wasn't sure: The further you get, the more inconvenient it becomes to add.
This one wouldn't become inconvenient to add since it would just be a kwarg in the OptimizationProblem IMO, so it should could just be slapped right in. @abhigupta768 take a look?
For calling the interface, it is a joke. But the "add-an-optimizer" internal API and the design of the return types will take a little bit of planning. Maybe I am just burnt from remembering what a pain it was for poor @pkofod to try to patch it onto Optim ex-post.
The most important places this will come up are in the specification of what is returned from the solution and what is passed to the optimizer. In particular':
(1) You do not want to call anything minimum and minimizer (e.g. https://github.com/SciML/GalacticOptim.jl/blob/master/test/rosenbrock.jl#L10)
- Keep more general terminology such as that used in MOI (e.g.
objective_value) or something like that or theobjectivein https://github.com/jump-dev/KNITRO.jl/blob/master/test/knitroapi.jl) - For the argmax or argmin, you could just use
.xor something like that if you wanted, as many of them do. Or I think that MOI usesprimalvalueor something like that, but maybe that is too fancy. (2) You want to be able to call the appropriate optimizer with the max vs. min sense and let them deal with the accounting of what to multiply by -1, etc. each optimizer has its own way to do this: - NLopt has
nlopt_set_min_objectiveandnlopt_set_max_objectivein https://nlopt.readthedocs.io/en/latest/NLopt_Reference/ - MOI has the objective sense passed in in the constructor
- raw KNITRO.jl API has
KNITRO.KN_set_obj_goal(kc, KNITRO.KN_OBJGOAL_MAXIMIZEl)etc.
On this, I think that it is reasonable to not always allow the sense = :max or whatever it is called in all optimizers. Most real optimizers would have it builtin, but for things like ADAM I cannot imagine anyone would ever want to use it as a maximizer. If the did, then they can manually do the change.
The one exception would be Optim support, which we should make sure to support maximize... but I think that should be in the Optim wrapper itself, and not something built into the outer stuff.
Right now we're just using Optim's output struct, but we'll need our own because Optim's does some weird things. that'll probably be the time to do this. We can support everything like ADAM as well: there's no harm since for those methods we'd just internally do the swap. I want the interface to be as uniform as possible among all
but for things like ADAM I cannot imagine anyone would ever want to use it as a maximizer
I have a major application 😆
Yeah, the Optim output struct is exactly the wrong design for this and what I am worried about.
Sounds good to me. As long as you leave dealing with the objective sense to the underlying optimizers (and don't do it yourself at the highest level in GalacticOptim) it is fine for me. Also, don't forget that there is at least one other type of objective sense than just max and min (e.g. passing in feasibility to simply ignore an objective, as in https://jump.dev/MathOptInterface.jl/dev/apireference/#MathOptInterface.ObjectiveSense ), so it shouldn't be hardcoded to a binary.
BTW, this is often a good way to solve complicated systems of equations and/or inequalities. You call an optimizer with the feasibility sense and it will just find any feasible point. Then you get all of the heuristics built into the optimizers for feasibility.
We should add sense in the OptimizationProblem. We should add a few enums to SciMLBase: call them MaxSense and MinSense, and then make the solve dispatches use prob.sense appropriately. Then we should change minimum to either solution or x, or result. .u is what the resulting optimized arguments/parameters are, so .x might make sense to just be generic and document.
@Vaibhavdixit02 what do you think of that design?
Hah! That's exactly how I have done it down to naming of the enum's elements, I forgot to push though and will be away from the computer until tomorrow so will create the PR then.
just doing my biannual check on this. Is there any movement on making the sense work across relevant optimizers? It sounded like it was close before?
Looks like sense is implemented in https://github.com/SciML/GalacticOptim.jl/blob/26904bd17e31b0fc47b03486577e494d35136a66/src/solve/nlopt.jl#L119 As well as the MOI one.
But not sure the solution type has been fixed up? https://github.com/SciML/SciMLBase.jl/blob/master/src/solutions/optimization_solutions.jl#L7
Yeah that's where it's at. It's implemented for some optimizers but the renaming hasn't happened.
Thanks @ChrisRackauckas It looks like it is in Optim already in https://github.com/SciML/GalacticOptim.jl/blob/master/src/solve/optim.jl#L88 so at that point I think there is decent enough coverage on optimizers.
Given that you have decided on the design above, it seems like this would just be going through and changing the name in SciMLBase and update it in GalacticOptim and its tests?
If so, then I think that we could do it on our side and submit a PR if there were no other gotchas.
@jlperla yes I think that's all that should be needed, please go ahead and get the PR up and we can wrap this up!
Just curious: Is there any particular reason to have these ObjSenses defined in Optimization.jl itself (https://github.com/SciML/Optimization.jl/blob/70085abb1c7a1707cffd979d5cb2d2ed72bb3dc4/src/Optimization.jl#L16) rather than SciMLBase.jl? I wanted to make use of it in another package and it seemed like it would be more natural to go there, so e.g. having Optimization.jl just to have the senses is not needed.
No reason, infact the intention was there to move it there and remove it from Optimization. Can you do the PR and we can do it right away?
@Vaibhavdixit02 Done. See #438 and SciML/SciMLBase.jl#333.
@Vaibhavdixit02 @ValentinKaisermayer Let's try to close this one out. I think the last thing is that we still have optsol.minimum. Let's choose a name here: optsol.result or optsol.optima? See the discussion above. Let's do that, deprecate .minimum, and close it out.
MOI calls it objective_value
That seems a bit odd though, I would expect that to be the value of the objective which is u
Isn't u the solution vector?
oh wait yes. True, I had it backwards haha. Yes, so sol.objective?
u* = argmin f(u), s.t. h(u) = 0, g(u) <= 0
I find objective not precise enough, it really is f but not it's value f(u*).
But I guess sol.objective is somewhat clear, since it is from a solution.
- https://github.com/SciML/Optimization.jl/pull/446
- https://github.com/SciML/SciMLBase.jl/pull/340
There you go @jlperla. A little late, but better than never haha.
Thanks @ValentinKaisermayer for pushing this over the finish line. You've a ton of good stuff recently!