Add emptycopy

[blegat]

This is needed to implement Base.copy in JuMP. (see jump-dev/JuMP.jl#1300).

[codecov-io]

Codecov Report

Merging #387 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   96.35%   96.37%   +0.02%     
==========================================
  Files          39       39              
  Lines        4913     4913              
==========================================
+ Hits         4734     4735       +1     
+ Misses        179      178       -1

Impacted Files	Coverage Δ
src/MathOptInterface.jl	100% <ø> (ø)	⬆️
src/Utilities/functions.jl	97.4% <0%> (+0.51%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13489d8...217321d. Read the comment docs.

[mlubin]

What is exactly is copied over, if anything? Solver parameters?

[blegat]

No, nothing at all. The solver parameters will be copied if it is followed by copy! (as we will do in JuMP).

[mlubin]

What about the license environment?

[blegat]

What is the issue with license environment ?
Maybe the easiest is to say that JuMP uses this by doing emptycopy followed by copy! so everything that will not be carried over by copy! need to be copied by emptycopy.

[mlubin]

Gurobi has an Env that needs to be copied when you create a new model. The Env has both license information and solver parameters.

[chriscoey]

bump

[blegat]

emptycopy is not needed anymore thanks to JuMP factories

[mlubin]

I think this is a useful function to have, but we're blocked until we define a clear concept for what is copied over to the empty model and how licences should be handled.

[odow]

If this is still useful, should I open an issue? Or can this PR be closed without one.

[blegat]

Not sure, we could just close, there is no need for it even if it could be useful

[mlubin]

I vote for reviving this proposal and removing OptimizerFactories at the JuMP level. Optimizer factories don't address the use case where MOI-level solvers need to take as input solver X and create multiple instances of this solver.

[blegat]

An argument for factories is that when you do

model = Model(Solver.Optimizer())

and internally it does

function Model(opt::Optimizer)
new_opt = MOI.empty_copy(opt)
return Model(..., new_opt, ...)
end

It seems less clean because the user has to create an optimizer that is not even used.
We cannot check isempty and use it if it is empty because a user might create an empty optimizer and give it to two models.

The counter-argument is that an empty optimizer object is light-weight so it does not matter.

[odow]

Why not define MOI solver factories?

[mlubin]

Yes, we could do that also. The underlying issue is that most modeling interfaces would typically use strings or enums to select solvers because they have a finite number of solvers available. We don't have that luxury.

[mlubin]

We can make the problem a bit simpler by normalizing the constructors for optimizers. For example, suppose we say that every optimizer needs to have a constructor like:

function MyOptimizer(solver_parameters::Array{Pair{String, Any}})
...
end

Then we can instantiate an optimizer given its type and its list of parameters (which is plain data). No need for a closure. No OptimizerFactory concept necessary.

In JuMP this could look like:

model = Model(optimizer=Ipopt.Optimizer, solve_parameters=["print_level" => 10])
...
set_optimizer(Gurobi.Optimizer, solve_parameters=["env" => my_env, "NodeLimit" => 10])
...
set_optimizer(Pajarito.Optimizer, solve_parameters=["mip_optimizer" => Gurobi.Optimizer, "mip_parameters" => ["NodeLimit" => 10]])

Note I chose to use strings instead of symbols because parameter names aren't identifiers (http://www.juliaopt.org/JuMP.jl/v0.19.0/style/#@enum-vs.-Symbol-1). Ipopt has parameter names with periods in them, for example.

Another option is to avoid taking parameters in the constructor when they're not needed at construction time, e.g.,

set_optimizer(model, Ipopt.Optimizer)
set_solve_parameter(model, "print_level", 10)

Some values like license contexts are needed at construction time, but most parameters are not. You then wouldn't have long awkward lists passed to solve_parameters=. The current way we do things is also unnecessarily awkward because it's confusing how to change a parameter after the optimizer has been created, when actually the underlying solvers usually support this directly.

[blegat]

Why not define MOI solver factories?

That would work

Another option is to avoid taking parameters in the constructor when they're not needed at construction time,

Setting parameters after model creation is not an option if you give a factory to a function which creates a model internally. You can solve it with closure by creating a closure that creates the optimizer, set parameters and return it but this is painful

[mlubin]

Setting parameters after model creation is not an option if you give a factory to a function which creates a model internally. You can solve it with closure by creating a closure that creates the optimizer, set parameters and return it but this is painful

I'm proposing this:

function instantiate_solver(optimizer_type, constructor_kwargs, solve_params)
    optimizer = optimizer_type(;constructor_kwargs...)
    for (k, v) in solve_params
        set(optimizer, RawParameter(k), v)
    end
    return optimizer
end

You pass optimizer_type, constructor_kwargs, and solve_paramers to anyone who needs to instantiate a solver. (You could possibly group these three fields in a struct.) There are no closures involved.

[odow]

Optimizer factories don't address the use case where MOI-level solvers need to take as input solver X and create multiple instances of this solver.

Defining MOI solver factories fixes this:

Juniper.Optimizer(mip_solver = MOI.with_optimizer(Gurobi.Optimizer, OutputFlag=0))

[mlubin]

I don't support moving OptimizerFactory as-is to MOI. It's a confusing data structure to work with. Look at the definition:
https://github.com/JuliaOpt/JuMP.jl/blob/f526373863cebe89a5a9f95a496ea8192378c9ae/src/JuMP.jl#L81-L89
(See also @kaarthiksundar's recent questions on the gitter channel on how to access and manipulate the solve parameters.)

I'm more convinced now that we should stop using constructor keyword arguments as the main way to pass solver-specific parameters. Using something like set(optimizer, RawParameter("some_gurobi_parameter"), ...) is less complicated and more consistent with MOI. You can pass collections of parameters and values separately from the mechanism for instantiating a new optimizer.

[blegat]

I like the idea of simplifying the optimizer constructor. For most solvers, that means that the constructors will now be empty as they don't need any parameter at construction time.

However, asking for optimizer_type to be a concrete type and preventing positional argument in the constructor is probably too restrictive, take for instance

with_optimizer(MOIU.MockOptimizer, JuMP._MOIModel{Float64}())

Note that if we do optimizer_type::Union{DataType, UnionAll} then we might as well do optimizer_type::Union{DataType, UnionAll, Function} or optimizer_type::Any in which case and then it's still a closure.
I don't see any issue with the closure though. Moving the OptimizerFactory to MOI and splitting kw args in constructor_kwargs and solve_params seems to me to be the option.

[mlubin]

I like the idea of simplifying the optimizer constructor. For most solvers, that means that the constructors will now be empty as they don't need any parameter at construction time.

Exactly.

I don't see any issue with the closure though. Moving the OptimizerFactory to MOI and splitting kw args in constructor_kwargs and solve_params seems to me to be the option.

I'd argue to just delete the OptimizerFactory object and when we need to, ask for "an object that when called with zero arguments returns a new Optimizer". Most of the time this will be the name of the optimizer itself, e.g., Gurobi.Optimizer. Advanced users can write () -> Gurobi.Optimizer(env), which is more transparent than with_optimizer(Gurobi.Optimizer, env).

The motivation for with_optimizer was to avoid scaring away first-time users of JuMP when they see set_optimizer(() -> IpoptOptimizer(print_level=1)). If the common case doesn't need explicit closures anymore, then this is no longer a concern.

[blegat]

Moving it to v0.10. If someone is willing to do this in the next days, we can move it back to v0.9.

[odow]

Can this be closed since we now have OptimizerWithAttributes?

[mlubin]

SGTM