Copy vector of variable before giving it to ScalarAffineFunction

[blegat]

We do push!(v, z) later so if we consider that v is owned by the function (since it does not copy), we need to copy before giving it

[mlubin]

Mmm. This needs a discussion on whether solvers can modify input (JuliaOpt/MathProgBase.jl#43). I'd tend to say that we should ask solvers not to modify the input.

[odow]

Solvers should never modify input. Otherwise you might be in the position where Gurobi does, but Clp doesn't. That could lead to a change in behaviour just by changing the solver...

[blegat]

Since MOI is low-level we can ask more cautiousness to the user to avoid unnecessary copy.
I would be tempted to let the solver modify it. It might do that if we call modifyconstraint later.
However it seems more reasonable to say that no one can modify the input: not the solver and not the user.

[mlubin]

@blegat, I think it's a corner case that solvers would want to keep the original vectors and modify them directly. Usually I would expect MOI wrappers to copy the input into a different data structure (e.g., loading it into the solver's API). I agree with @odow here.

[mlubin]

Another point is that when loading a lot of linear constraints, users might want to re-use the same input vectors to avoid allocating new ones for each constraint.

[blegat]

So that means that every time JuMP will add a constraint to Instance, the instance will copy it. That is very inefficient.

[IssamT]

Use case:

• create vector : v
• create function : f
• add constraint : f in s1
• modify vector v
• add constraint : f in s2

With the current implementation f has changed no? if yes then there isn't only a problem on the solver side but also on the user side if the function is pointing on the same vector that the user has provided.

I agree with @odow that solvers should not modify input data. but if the user is planning to modify v after giving it to a function he should provide a copy to the function. In the case the user will not modify the vector then he can provide it without copying it. In C++ I would put the argument of the function as a reference on a const vector.

EDIT
The user doesn't need to copy v provided to f, even if v will be modified after if he is not planning to use f after modifying v.

[blegat]

I have update the PR into a less controversial change : I do not change v the vector of variables in the user side.

[IssamT]

How about modifying functions to use W̶r̶a̶p̶p̶e̶r̶s̶ ̶o̶f̶ Tuples instead of vectors? If I am not wrong, this way a modification will be possible but only through modifying the whole Tuple.

[blegat]

The conversion from Vector to tuple is not type stable so it might be inefficient.

[IssamT]

But why would you use Vector if the functions are using Tuple?
The user will have:

v = (x,y)
# create a function using that v
v = (v... , z)
# create another function using the new v

This way the push! is denied by design.

[blegat]

This would require JuMP to store his variables as an NTuple. If we want JuMP.Model to have concrete types, it will need to have N as a type parameter which means that @variable would need to change the type of the model.
In general, it seems that requiring every user of optimization to use tuple is rather restrictive.

[mlubin]

Tuples are not an appropriate replacement for arbitrary-length vectors. We could easily have an affine function with 1,000,000 coefficients:

julia> tuple(ones(1000000)...)

signal (11): Segmentation fault

[mlubin]

So that means that every time JuMP will add a constraint to Instance, the instance will copy it. That is very inefficient.

We can make some optimizations between JuMP and its internal instance if needed. I don't think that should guide the interface design.

I would propose that we view the vectors passed though MOI as temporary and belonging to the users, not to solvers. Solvers should not assume that the contents of the vectors will not change in the future. Users may assume that solvers will not modify the vectors.

I believe we agree that solvers should not modify the input vectors because that creates a whole class of potential bugs. If we say that users cannot modify the input vectors after providing them to the solver, then we're removing the possibility of reusing the vectors later. I think the following is a pretty reasonable pattern for the user to do:

variables = Vector{VariableReference}(0)
coefficients = Vector{Float64}(0)
for each constraint we want to add
    resize variables and coefficients
    fill variables and coefficients with values
    addconstraint!(m, ScalarAffineFunction(variables, coefficients, constant), ...)
end

If we disallow the user to modify vectors afterwords, we have to create new Vector{Float64} and Vector{VariableReference} objects for each constraint, which could incur GC cost.

[blegat]

The issue is that SOI's solvers, I store everything in MOIU's Instance type and only load it into the solver when optimize! is called. The reason is that these solvers need to first know the size of the problem before you can fill the coefficients of the constraints, ...
If I copy each function inputted before giving it to the instance, it's very inefficient.

Why not having a performance hint (see #63) saying that the input vectors will not be modified by the user ? Most solvers would not care but since JuMP will set this hint, SOI's solvers will not do extra copies when called through JuMP.

[mlubin]

Is it really that inefficient? Could you quantify this?

[blegat]

I will make me sleep bad when I think about it and I will not enjoy using these solvers for sum of squares thinking about the copies that are being done :D

[mlubin]

I mean quantify the actual performance penalty, not the psychological damage :)

[mlubin]

I'd like to see an example where this copying has a performance hit of greater than 5% of the solve time.

[blegat]

If the constraints are given in a huge vector constraint then at the moment of the copy there is twice more memory used than required and this might cause failure. Le 20 juil. 2017 23:38, "Miles Lubin" <notifications@github.com> a écrit :

…

I'd like to see an example where this copying has a performance hit of greater than 5% of the solve time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_-jUIL3-Lsa-O2EgEO-EnnlfBhG77hks5sP8i5gaJpZM4OdPCj> .

[blegat]

By the way, JuMP may also use this performance hint with Instance. This shows that this performance hint may be useful for users of Instance Le 21 juil. 2017 00:00, "Benoît Legat" <benoit.legat@gmail.com> a écrit :

…

If the constraints are given in a huge vector constraint then at the moment of the copy there is twice more memory used than required and this might cause failure. Le 20 juil. 2017 23:38, "Miles Lubin" ***@***.***> a écrit : > I'd like to see an example where this copying has a performance hit of > greater than 5% of the solve time. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#67 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AA_-jUIL3-Lsa-O2EgEO-EnnlfBhG77hks5sP8i5gaJpZM4OdPCj> > . >

[mlubin]

If the constraints are given in a huge vector constraint then at the moment
of the copy there is twice more memory used than required and this might
cause failure.

There will also be a moment when the same amount of memory is used while the wrapper is copying the problem into the C interface, so wouldn't you have a failure at that point also?

[blegat]

Closing since this will not be merged. The discussion for the performance hint remains open, e.g. JuliaOpt/MathOptInterfaceBridges.jl#2