Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operator count inconsistency with JuMP #57

Closed
jd-lara opened this issue Jul 15, 2019 · 9 comments
Closed

Operator count inconsistency with JuMP #57

jd-lara opened this issue Jul 15, 2019 · 9 comments

Comments

@jd-lara
Copy link
Contributor

jd-lara commented Jul 15, 2019

When I run sum(GAEv) the operator counter in JuMP stays in 0 since it performs all the operations using add_to_expression!. However, if I run sum(PAE) the operator counter is different than 0.

Some algebra method might be causing a call to a + operator instead of add_to_expression.

@jd-lara
Copy link
Contributor Author

jd-lara commented Aug 27, 2019

@joaquimg here is an example of the issue.

using JuMP
using Random
m = Model() 
I = 1:200
J = 1:200
vals = rand(length(I))
@variable(m, x[I, J] >= 0)
expression_container = JuMP.Containers.DenseAxisArray{JuMP.GenericAffExpr{Float64, VariableRef}}(undef, I)
for i in J
    expression_container[i] = sum(x[i,:]) - vals[i]
end
@constraint(m, con, sum(expression_container) >= 0);
m.operator_counter
0
using ParameterJuMP
mwp = ModelWithParams()
I = 1:200
J = 1:200
vals = rand(length(I))
param_vals = add_parameters(mwp, vals)
@variable(mwp, x[I, J] >= 0)
expression_container = JuMP.Containers.DenseAxisArray{ParameterJuMP.DoubleGenericAffExpr{Float64, VariableRef, ParameterRef}}(undef, I)
for i in J
    expression_container[i] = sum(x[i,:]) - param_vals[i]
end
@constraint(mwp, con, sum(expression_container) >= 0);
mwp.operator_counter
548

For larger models, the way the expression algebra is implemented leads to the following warning in JuMP.

Warning: The addition operator has been used on JuMP expressions a large number of times. This warning is safe to ignore but may indicate that model generation is slower than necessary. For performance reasons, you should not add expressions in a loop. Instead of x += y, use add_to_expression!(x,y) to modify x in place. If y is a single variable, you may also use add_to_expression!(x, coef, y) for x += coef*y.

@jd-lara
Copy link
Contributor Author

jd-lara commented Aug 30, 2019

bump @joaquimg and @blegat about this. Any hints on how to address it?

@blegat
Copy link
Member

blegat commented Aug 31, 2019

In sum(x[i,:]) - param_vals[i] you call Base.- instead of add_to_expression, isn't that what's causing the counter to increase ?

@jd-lara
Copy link
Contributor Author

jd-lara commented Aug 31, 2019

I think if that were the case the counter would be other than 0 in the JuMP example

@jd-lara
Copy link
Contributor Author

jd-lara commented Sep 3, 2019

I solved it by calling .destructive_add! when adding PAE + variable.

@jd-lara jd-lara closed this as completed Sep 3, 2019
@blegat
Copy link
Member

blegat commented Sep 3, 2019

So it was not in the sum ?

@jd-lara
Copy link
Contributor Author

jd-lara commented Sep 3, 2019

Yeah the sums of PAE are not adding counts.

@joaquimg
Copy link
Collaborator

joaquimg commented Sep 4, 2019

You are calling destructive_add explicitly?
Where that is called in the example?
Doesn't looks like there is any PAE +
variable.
The problem is GAEv + param?

@jd-lara
Copy link
Contributor Author

jd-lara commented Sep 4, 2019

After some exploration, I realized what actually fixed the problem was the JuMP update to 0.20.0 and not the use of destructive_add, my guess is that it was this PR jump-dev/JuMP.jl#1902.

This is the test code and an example of the final implementation I did in PowerSimulations.

using ParameterJuMP
using JuMP
mwp = ModelWithParams()
I = 1:200
J = 1:200
vals = rand(length(I))
param_vals = add_parameters(mwp, vals)
@variable(mwp, x[I, J] >= 0)
expression_container = JuMP.Containers.DenseAxisArray{ParameterJuMP.DoubleGenericAffExpr{Float64, VariableRef, ParameterRef}}(undef, I)
for i in J
    expression_container[i] = sum(x[i,:])
    add_to_expression!(expression_container[i], - param_vals[i])
end
@constraint(mwp, con, sum(expression_container) >= 0);
mwp.operator_counter

In JuMP 0.19.2 and ParameterJuMP v0.1.1 this code results in 348 operations and in JuMP 0.20.0 and ParameterJuMP v0.1.2 results in 0.

Sorry for the confusion, it wasn't clear what might have fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants