Optimization.jl icon indicating copy to clipboard operation
Optimization.jl copied to clipboard

Add objective sense into the front-end

Open jlperla opened this issue 5 years ago • 14 comments

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.

jlperla avatar May 14 '20 15:05 jlperla

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

jlperla avatar May 14 '20 16:05 jlperla

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.

jlperla avatar Jul 15 '20 15:07 jlperla

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?

ChrisRackauckas avatar Jul 15 '20 15:07 ChrisRackauckas

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 the objective in https://github.com/jump-dev/KNITRO.jl/blob/master/test/knitroapi.jl)
  • For the argmax or argmin, you could just use .x or something like that if you wanted, as many of them do. Or I think that MOI uses primalvalue or 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_objective and nlopt_set_max_objective in 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.

jlperla avatar Jul 15 '20 16:07 jlperla

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 😆

ChrisRackauckas avatar Jul 15 '20 16:07 ChrisRackauckas

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.

jlperla avatar Jul 15 '20 16:07 jlperla

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.

ChrisRackauckas avatar Jul 14 '21 14:07 ChrisRackauckas

@Vaibhavdixit02 what do you think of that design?

ChrisRackauckas avatar Jul 14 '21 14:07 ChrisRackauckas

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.

Vaibhavdixit02 avatar Jul 14 '21 14:07 Vaibhavdixit02

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?

jlperla avatar Jan 01 '22 05:01 jlperla

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

jlperla avatar Jan 01 '22 05:01 jlperla

Yeah that's where it's at. It's implemented for some optimizers but the renaming hasn't happened.

ChrisRackauckas avatar Jan 01 '22 19:01 ChrisRackauckas

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 avatar Jan 03 '22 18:01 jlperla

@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!

Vaibhavdixit02 avatar Jan 04 '22 02:01 Vaibhavdixit02

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.

DanielVandH avatar Dec 06 '22 11:12 DanielVandH

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 avatar Dec 06 '22 12:12 Vaibhavdixit02

@Vaibhavdixit02 Done. See #438 and SciML/SciMLBase.jl#333.

DanielVandH avatar Dec 07 '22 08:12 DanielVandH

@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.

ChrisRackauckas avatar Dec 11 '22 12:12 ChrisRackauckas

MOI calls it objective_value

ValentinKaisermayer avatar Dec 11 '22 13:12 ValentinKaisermayer

That seems a bit odd though, I would expect that to be the value of the objective which is u

ChrisRackauckas avatar Dec 11 '22 13:12 ChrisRackauckas

Isn't u the solution vector?

ValentinKaisermayer avatar Dec 11 '22 13:12 ValentinKaisermayer

oh wait yes. True, I had it backwards haha. Yes, so sol.objective?

ChrisRackauckas avatar Dec 11 '22 13:12 ChrisRackauckas

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*).

ValentinKaisermayer avatar Dec 11 '22 13:12 ValentinKaisermayer

But I guess sol.objective is somewhat clear, since it is from a solution.

ValentinKaisermayer avatar Dec 11 '22 13:12 ValentinKaisermayer

  • https://github.com/SciML/Optimization.jl/pull/446
  • https://github.com/SciML/SciMLBase.jl/pull/340

ValentinKaisermayer avatar Dec 11 '22 13:12 ValentinKaisermayer

There you go @jlperla. A little late, but better than never haha.

ChrisRackauckas avatar Dec 12 '22 12:12 ChrisRackauckas

Thanks @ValentinKaisermayer for pushing this over the finish line. You've a ton of good stuff recently!

ChrisRackauckas avatar Dec 12 '22 12:12 ChrisRackauckas