Skip to content

Commit

Permalink
Remove functions which worked on Variables
Browse files Browse the repository at this point in the history
We can't treat Variables and Expressions the same if we want all the functions to be in-place. Using a variable will work but we can't reassign the new expression to the original variable. We could consider other helper functions which take in variables and turn them into expressions. However, in many cases it'll be easier to use add_similar_to_expression!() with the variable as the second argument
  • Loading branch information
RuaridhMacd committed Jan 18, 2024
1 parent ed8a88e commit 735701e
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 73 deletions.
46 changes: 1 addition & 45 deletions src/model/expression_manipulation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -101,31 +101,6 @@ function add_similar_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, di
return nothing
end

# Version for single element
function add_similar_to_expression!(expr1::GenericVariableRef{C}, expr2::V) where {C,V}
add_to_expression!(expr1, expr2)
return nothing
end

@doc raw"""
add_similar_to_expression!(expr1::AbstractArray{GenericVariableRef{C}, dim1}, expr2::AbstractArray{V, dim2}) where {C,V,dim1,dim2}
Version of add_similar_to_expression() for variables.
Expressions containing only variables are automatically converted to variables by default.
This uses a dummy coefficient to convert the variables to expressions.
"""
function add_similar_to_expression!(expr1::AbstractArray{GenericVariableRef{C}, dim1}, expr2::AbstractArray{V, dim2}) where {C,V,dim1,dim2}
# This is defined for Arrays of different dimensions
# despite the fact it will definitely throw an error
# because the error will tell the user / developer
# the dimensions of both arrays
check_sizes_match(expr1, expr2)
for i in eachindex(expr1)
add_to_expression!(1 * expr1[i], expr2[i])
end
return nothing
end

###### ###### ###### ###### ###### ######
# Element-wise addition of one term into an expression
# Both arrays must have the same dimensions
Expand All @@ -150,26 +125,6 @@ function add_term_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dims}
return nothing
end

# Version for single element
function add_term_to_expression!(expr1::GenericVariableRef{C}, expr2::V) where {C,V}
add_to_expression!(1 * expr1, expr2)
return nothing
end

@doc raw"""
add_term_to_expression!(expr1::AbstractArray{GenericVariableRef{C}, dims}, expr2::V) where {C,V,dims}
Version of add_term_to_expression() for variables.
Expressions containing only variables are automatically converted to variables by default.
This uses a dummy coefficient to convert the variables to expressions.
"""
function add_term_to_expression!(expr1::AbstractArray{GenericVariableRef{C}, dims}, expr2::V) where {C,V,dims}
for i in eachindex(expr1)
add_to_expression!(1 * expr1[i], expr2)
end
return nothing
end

###### ###### ###### ###### ###### ######
# Check that two arrays have the same dimensions
###### ###### ###### ###### ###### ######
Expand Down Expand Up @@ -212,6 +167,7 @@ end
sum_expression(expr::AbstractArray{C, dims}) where {C,dims} :: C
Sum an array of expressions into a single expression and return the result.
We're using errors from add_to_expression!() to check that the types are compatible.
"""
function sum_expression(expr::AbstractArray{C, dims}) :: AffExpr where {C,dims}
# check_addable_to_expr(C,C)
Expand Down
28 changes: 0 additions & 28 deletions test/expression_manipulation_test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,6 @@ function sum_combo_expr()
return true
end

function add_term_disjoint_var()
EP = setup_sum_model()
try
GenX.add_term_to_expression!(EP[:x], 6.0)
catch
return false
end
return true
end

function add_term_dense_var()
EP = setup_sum_model()
try
GenX.add_term_to_expression!(EP[:y], 6.0)
catch
return false
end
return true
end

let
EP = Model(HiGHS.Optimizer)

Expand Down Expand Up @@ -129,21 +109,13 @@ let
GenX.add_term_to_expression!(EP[:large_expr], AffExpr(3.0))
@test EP[:large_expr][100] == test_var[100] + 22.0

# Test add_term_to_expression! for variable
@variable(EP, single_var >= 0)
GenX.add_term_to_expression!(EP[:large_expr], single_var)
@test EP[:large_expr][100] == test_var[100] + 22.0 + single_var

# Test sum_expression
@test sum_dense_var() == true
@test sum_disjoint_var() == true
@test sum_dense_expr() == true
@test sum_disjoint_expr() == true
@test sum_combo_expr() == true

# Test add_term_to_expression! on variables
@test add_term_dense_var() == true
@test add_term_disjoint_var() == true
end

###### ###### ###### ###### ###### ###### ######
Expand Down

0 comments on commit 735701e

Please sign in to comment.