From 19864f64e0c8df5c0e1c78e897f816dd152d4482 Mon Sep 17 00:00:00 2001 From: Ruaridh Macdonald Date: Tue, 14 Nov 2023 16:40:38 -0500 Subject: [PATCH 1/5] Update expression manipulation and add more tests - Added docstrings. Still need to add to the documentation - Simplified the versions further - Added add_to_expression for variables, using Jacob's 1 * VarRef trick --- src/model/expression_manipulation.jl | 115 ++++++++++++++++++++------- test/expression_manipulation_test.jl | 67 ++++++++++++++++ 2 files changed, 154 insertions(+), 28 deletions(-) diff --git a/src/model/expression_manipulation.jl b/src/model/expression_manipulation.jl index 20b9474ceb..d043d26449 100644 --- a/src/model/expression_manipulation.jl +++ b/src/model/expression_manipulation.jl @@ -2,11 +2,13 @@ # Create dense arrays of expressions filled with zeros to be added to later ###### ###### ###### ###### ###### ###### +# Single element version function create_empty_expression!(EP::Model, exprname::Symbol) EP[exprname] = AffExpr(0.0) return nothing end +# Vector version, to avoid needing to wrap the dimension in a tuple or array function create_empty_expression!(EP::Model, exprname::Symbol, dim1::Int64) temp = Array{AffExpr}(undef, dim1) fill_with_zeros!(temp) @@ -14,6 +16,15 @@ function create_empty_expression!(EP::Model, exprname::Symbol, dim1::Int64) return nothing end +@doc raw""" + create_empty_expression!(EP::Model, exprname::Symbol, dims::NTuple{N, Int64}) where N + +Create an dense array filled with zeros which can be altered later. +Other approaches to creating zero-filled arrays will often return an array of floats, not expressions. +This can lead to errors later if a method can only operate on expressions. + +We don't currently have a method to do this with non-contiguous indexing. +""" function create_empty_expression!(EP::Model, exprname::Symbol, dims::NTuple{N, Int64}) where N temp = Array{AffExpr}(undef, dims) fill_with_zeros!(temp) @@ -21,8 +32,8 @@ function create_empty_expression!(EP::Model, exprname::Symbol, dims::NTuple{N, I return nothing end +# Version with the dimensions wrapped in an array. This requires slightly more memory than using tuples function create_empty_expression!(EP::Model, exprname::Symbol, dims::Vector{Int64}) - # The version using tuples is slightly less memory temp = Array{AffExpr}(undef, dims...) fill_with_zeros!(temp) EP[exprname] = temp @@ -33,15 +44,28 @@ end # Fill dense arrays of expressions with zeros or a constant ###### ###### ###### ###### ###### ###### -function fill_with_zeros!(arr::Array{GenericAffExpr{C,T}, dims}) where {C,T,dims} +@doc raw""" + fill_with_zeros!(arr::Array{GenericAffExpr{C,T}, dims}) where {C,T,dims} + +Fill an array of expressions with zeros in-place. +""" +function fill_with_zeros!(arr::AbstractArray{GenericAffExpr{C,T}, dims}) where {C,T,dims} for i::Int64 in eachindex(IndexLinear(), arr)::Base.OneTo{Int64} arr[i] = AffExpr(0.0) end return nothing end -function fill_with_const!(arr::Array{GenericAffExpr{C,T}, dims}, con::Real) where {C,T,dims} - for i::Int64 in eachindex(IndexLinear(), arr)::Base.OneTo{Int64} +@doc raw""" + fill_with_const!(arr::Array{GenericAffExpr{C,T}, dims}) where {C,T,dims} + +Fill an array of expressions with the specified constant, in-place. + +In the future we could expand this to non AffExpr, using GenericAffExpr +e.g. if we wanted to use Float32 instead of Float64 +""" +function fill_with_const!(arr::AbstractArray{GenericAffExpr{C,T}, dims}, con::Real) where {C,T,dims} + for i in eachindex(arr) arr[i] = AffExpr(con) end return nothing @@ -52,31 +76,52 @@ end # Both arrays must have the same dimensions ###### ###### ###### ###### ###### ###### +# Version for single element function add_similar_to_expression!(expr1::GenericAffExpr{C,T}, expr2::V) where {C,T,V} add_to_expression!(expr1, expr2) return nothing end -function add_similar_to_expression!(expr1::Array{GenericAffExpr{C,T}, dim1}, expr2::Array{V, dim2}) where {C,T,V,dim1,dim2} +@doc raw""" + add_similar_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dim1}, expr2::AbstractArray{V, dim2}) where {C,T,V,dim1,dim2} + +Add an array of some type `V` to an array of expressions, in-place. +This will work on JuMP DenseContainers which do not have linear indexing from 1:length(arr). +However, the accessed parts of both arrays must have the same dimensions. +""" +function add_similar_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dim1}, expr2::AbstractArray{V, dim2}) where {C,T,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(IndexLinear(), expr1) + for i in eachindex(expr1) add_to_expression!(expr1[i], expr2[i]) end return nothing end -function add_similar_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dim1}, expr2::AbstractArray{V, dim2}) where {C,T,V,dim1,dim2} +# 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!(expr1[i], expr2[i]) + add_to_expression!(1 * expr1[i], expr2[i]) end return nothing end @@ -86,18 +131,18 @@ end # Both arrays must have the same dimensions ###### ###### ###### ###### ###### ###### +# Version for single element function add_term_to_expression!(expr1::GenericAffExpr{C,T}, expr2::V) where {C,T,V} add_to_expression!(expr1, expr2) return nothing end -function add_term_to_expression!(expr1::Array{GenericAffExpr{C,T}, dims}, expr2::V) where {C,T,V,dims} - for i in eachindex(IndexLinear(), expr1) - add_to_expression!(expr1[i], expr2) - end - return nothing -end +@doc raw""" + add_term_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dims}, expr2::V) where {C,T,V,dims} +Add an entry of type `V` to an array of expressions, in-place. +This will work on JuMP DenseContainers which do not have linear indexing from 1:length(arr). +""" function add_term_to_expression!(expr1::AbstractArray{GenericAffExpr{C,T}, dims}, expr2::V) where {C,T,V,dims} for i in eachindex(expr1) add_to_expression!(expr1[i], expr2) @@ -109,6 +154,12 @@ end # Check that two arrays have the same dimensions ###### ###### ###### ###### ###### ###### +@doc raw""" + check_sizes_match(expr1::AbstractArray{C, dim1}, expr2::AbstractArray{T, dim2}) where {C,T,dim1, dim2} + +Check that two arrays have the same dimensions. +If not, return an error message which includes the dimensions of both arrays. +""" function check_sizes_match(expr1::AbstractArray{C, dim1}, expr2::AbstractArray{T, dim2}) where {C,T,dim1, dim2} # After testing, this appears to be just as fast as a method for Array{GenericAffExpr{C,T}, dims} or Array{AffExpr, dims} if size(expr1) != size(expr2) @@ -118,27 +169,35 @@ function check_sizes_match(expr1::AbstractArray{C, dim1}, expr2::AbstractArray{T end end +@doc raw""" + check_addable_to_expr(C::DataType, T::DataType) + +Check that two datatype can be added using add_to_expression!(). Raises an error if not. + +This needs some work to make it more flexible. Right now it's challenging to use with GenericAffExpr{C,T} +as the method only works on the constituent types making up the GenericAffExpr, not the resulting expression type. +Also, the default MethodError from add_to_expression! is sometime more informative than the error message here. +""" +function check_addable_to_expr(C::DataType, T::DataType) + if !(hasmethod(add_to_expression!, (C,T))) + error("No method found for add_to_expression! with types $(C) and $(T)") + end +end + ###### ###### ###### ###### ###### ###### # Sum an array of expressions into a single expression ###### ###### ###### ###### ###### ###### -function sum_expression(expr::AbstractArray{C, dims}) where {C,dims} - # This appears to work just as well as a separate method for Array{AffExpr, dims} +@doc raw""" + sum_expression(expr::AbstractArray{C, dims}) where {C,dims} :: C + +Sum an array of expressions into a single expression and return the result. +""" +function sum_expression(expr::AbstractArray{C, dims}) :: AffExpr where {C,dims} + # check_addable_to_expr(C,C) total = AffExpr(0.0) for i in eachindex(expr) add_to_expression!(total, expr[i]) end return total -end - -function sum_expression(expr::AbstractArray{GenericAffExpr{C,T}, dims}) where {C,T,dims} - return _sum_expression(expr) -end - -function sum_expression(expr::AbstractArray{GenericVariableRef{C}, dims}) where {C,dims} - return _sum_expression(expr) -end - -function sum_expression(expr::AbstractArray{AbstractJuMPScalar, dims}) where {dims} - return _sum_expression(expr) end \ No newline at end of file diff --git a/test/expression_manipulation_test.jl b/test/expression_manipulation_test.jl index 1771f0d144..a902e0ba81 100644 --- a/test/expression_manipulation_test.jl +++ b/test/expression_manipulation_test.jl @@ -1,6 +1,66 @@ using JuMP using HiGHS +function setup_sum_model() + EP = Model(HiGHS.Optimizer) + @variable(EP, x[i=1:100,j=1:4:200]>=0) + @variable(EP, y[i=1:100,j=1:50]>=0) + @expression(EP, eX[i=1:100,j=1:4:200], 2.0*x[i,j]+i+10.0*j) + @expression(EP, eY[i=1:100,j=1:50], 3.0*y[i,j]+2*i+j) + @expression(EP, eZ[i=1:100,j=1:50], 2.0*x[i,(j-1)*4+1] + 4.0*y[i,j]) + return EP +end + +function sum_disjoint_var() + EP = setup_sum_model() + try + GenX.sum_expression(EP[:x]) + catch + return false + end + return true +end + +function sum_dense_var() + EP = setup_sum_model() + try + GenX.sum_expression(EP[:y]) + catch + return false + end + return true +end + +function sum_disjoint_expr() + EP = setup_sum_model() + try + GenX.sum_expression(EP[:eX]) + catch + return false + end + return true +end + +function sum_dense_expr() + EP = setup_sum_model() + try + GenX.sum_expression(EP[:eY]) + catch + return false + end + return true +end + +function sum_combo_expr() + EP = setup_sum_model() + try + GenX.sum_expression(EP[:eZ]) + catch + return false + end + return true +end + let EP = Model(HiGHS.Optimizer) @@ -53,6 +113,13 @@ let @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 end ###### ###### ###### ###### ###### ###### ###### From ed8a88e6c968b5298123f8ab70ee60b3b7980e8d Mon Sep 17 00:00:00 2001 From: Ruaridh Macdonald Date: Tue, 14 Nov 2023 16:54:06 -0500 Subject: [PATCH 2/5] VarRef version of add_term_to_expression! --- src/model/expression_manipulation.jl | 20 ++++++++++++++++++++ test/expression_manipulation_test.jl | 24 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/model/expression_manipulation.jl b/src/model/expression_manipulation.jl index d043d26449..b63f01e76b 100644 --- a/src/model/expression_manipulation.jl +++ b/src/model/expression_manipulation.jl @@ -150,6 +150,26 @@ 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 ###### ###### ###### ###### ###### ###### diff --git a/test/expression_manipulation_test.jl b/test/expression_manipulation_test.jl index a902e0ba81..3a16dd869a 100644 --- a/test/expression_manipulation_test.jl +++ b/test/expression_manipulation_test.jl @@ -61,6 +61,26 @@ 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) @@ -120,6 +140,10 @@ let @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 ###### ###### ###### ###### ###### ###### ###### From 735701e1fc848afbdd17d2866c5243bbc51edf26 Mon Sep 17 00:00:00 2001 From: Ruaridh Macdonald Date: Thu, 18 Jan 2024 10:16:46 -0500 Subject: [PATCH 3/5] Remove functions which worked on Variables 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 --- src/model/expression_manipulation.jl | 46 +--------------------------- test/expression_manipulation_test.jl | 28 ----------------- 2 files changed, 1 insertion(+), 73 deletions(-) diff --git a/src/model/expression_manipulation.jl b/src/model/expression_manipulation.jl index b63f01e76b..dd2cd86a36 100644 --- a/src/model/expression_manipulation.jl +++ b/src/model/expression_manipulation.jl @@ -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 @@ -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 ###### ###### ###### ###### ###### ###### @@ -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) diff --git a/test/expression_manipulation_test.jl b/test/expression_manipulation_test.jl index 3a16dd869a..c0a3fb8d39 100644 --- a/test/expression_manipulation_test.jl +++ b/test/expression_manipulation_test.jl @@ -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) @@ -129,11 +109,6 @@ 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 @@ -141,9 +116,6 @@ let @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 ###### ###### ###### ###### ###### ###### ###### From d4b234b280b72f0adef0ede5d8d6251d20eab8cd Mon Sep 17 00:00:00 2001 From: Luca Bonaldo <39280783+lbonaldo@users.noreply.github.com> Date: Thu, 1 Feb 2024 12:55:18 -0500 Subject: [PATCH 4/5] Update src/model/expression_manipulation.jl --- src/model/expression_manipulation.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/expression_manipulation.jl b/src/model/expression_manipulation.jl index 1a2c75eb9b..39e6c50d3e 100644 --- a/src/model/expression_manipulation.jl +++ b/src/model/expression_manipulation.jl @@ -45,7 +45,7 @@ end ###### ###### ###### ###### ###### ###### @doc raw""" - fill_with_zeros!(arr::Array{GenericAffExpr{C,T}, dims}) where {C,T,dims} + fill_with_zeros!(arr::AbstractArray{GenericAffExpr{C,T}, dims}) where {C,T,dims} Fill an array of expressions with zeros in-place. """ From e2699370d3775e38c1617bfc9c55a562fd3acc19 Mon Sep 17 00:00:00 2001 From: Luca Bonaldo <39280783+lbonaldo@users.noreply.github.com> Date: Thu, 1 Feb 2024 12:56:03 -0500 Subject: [PATCH 5/5] Update src/model/expression_manipulation.jl --- src/model/expression_manipulation.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/expression_manipulation.jl b/src/model/expression_manipulation.jl index 39e6c50d3e..fe4fe6e7be 100644 --- a/src/model/expression_manipulation.jl +++ b/src/model/expression_manipulation.jl @@ -57,7 +57,7 @@ function fill_with_zeros!(arr::AbstractArray{GenericAffExpr{C,T}, dims}) where { end @doc raw""" - fill_with_const!(arr::Array{GenericAffExpr{C,T}, dims}) where {C,T,dims} + fill_with_const!(arr::AbstractArray{GenericAffExpr{C,T}, dims}, con::Real) where {C,T,dims} Fill an array of expressions with the specified constant, in-place.