From d63fe6f70ee8da9246b1ede6bd0d8aa741ab6e57 Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 1 Nov 2024 10:41:16 +1300 Subject: [PATCH 1/5] Improve performance of haskey lookups --- src/MOI_wrapper.jl | 88 ++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl index 0dc36d1..3a34ae4 100644 --- a/src/MOI_wrapper.jl +++ b/src/MOI_wrapper.jl @@ -731,10 +731,11 @@ function MOI.get(model::Optimizer, ::MOI.ListOfVariableIndices) end function _info(model::Optimizer, key::MOI.VariableIndex) - if haskey(model.variable_info, key) - return model.variable_info[key] + info = get(model.variable_info, key, nothing) + if info === nothing + return throw(MOI.InvalidIndex(key)) end - return throw(MOI.InvalidIndex(key)) + return info end """ @@ -795,14 +796,13 @@ function MOI.get(model::Optimizer, ::Type{MOI.VariableIndex}, name::String) if model.name_to_variable === nothing _rebuild_name_to_variable(model) end - if haskey(model.name_to_variable, name) - variable = model.name_to_variable[name] - if variable === nothing - error("Duplicate name detected: $(name)") - end - return variable + variable = get(model.name_to_variable, name, missing) + if variable === nothing + error("Duplicate name detected: $(name)") + elseif variable === missing + return nothing # Variable with this name does not exist end - return nothing + return variable::MOI.VariableIndex end function MOI.get(model::Optimizer, ::MOI.VariableName, v::MOI.VariableIndex) @@ -1172,10 +1172,11 @@ function _info( c::MOI.ConstraintIndex{MOI.VariableIndex,<:Any}, ) var_index = MOI.VariableIndex(c.value) - if haskey(model.variable_info, var_index) - return _info(model, var_index) + info = get(model.variable_info, var_index, nothing) + if info === nothing + return throw(MOI.InvalidIndex(c)) end - return throw(MOI.InvalidIndex(c)) + return info end """ @@ -1197,40 +1198,46 @@ function MOI.is_valid( model::Optimizer, c::MOI.ConstraintIndex{MOI.VariableIndex,MOI.LessThan{Float64}}, ) - if haskey(model.variable_info, MOI.VariableIndex(c.value)) - info = _info(model, c) - return info.bound == _BOUND_LESS_THAN || - info.bound == _BOUND_LESS_AND_GREATER_THAN + info = get(model.variable_info, MOI.VariableIndex(c.value), nothing) + if info === nothing + return false end - return false + return info.bound == _BOUND_LESS_THAN || + info.bound == _BOUND_LESS_AND_GREATER_THAN end function MOI.is_valid( model::Optimizer, c::MOI.ConstraintIndex{MOI.VariableIndex,MOI.GreaterThan{Float64}}, ) - if haskey(model.variable_info, MOI.VariableIndex(c.value)) - info = _info(model, c) - return info.bound == _BOUND_GREATER_THAN || - info.bound == _BOUND_LESS_AND_GREATER_THAN + info = get(model.variable_info, MOI.VariableIndex(c.value), nothing) + if info === nothing + return false end - return false + return info.bound == _BOUND_GREATER_THAN || + info.bound == _BOUND_LESS_AND_GREATER_THAN end function MOI.is_valid( model::Optimizer, c::MOI.ConstraintIndex{MOI.VariableIndex,MOI.Interval{Float64}}, ) - return haskey(model.variable_info, MOI.VariableIndex(c.value)) && - _info(model, c).bound == _BOUND_INTERVAL + info = get(model.variable_info, MOI.VariableIndex(c.value), nothing) + if info === nothing + return false + end + return info.bound == _BOUND_INTERVAL end function MOI.is_valid( model::Optimizer, c::MOI.ConstraintIndex{MOI.VariableIndex,MOI.EqualTo{Float64}}, ) - return haskey(model.variable_info, MOI.VariableIndex(c.value)) && - _info(model, c).bound == _BOUND_EQUAL_TO + info = get(model.variable_info, MOI.VariableIndex(c.value), nothing) + if info === nothing + return false + end + return info.bound == _BOUND_EQUAL_TO end function MOI.get( @@ -1495,10 +1502,11 @@ function _info( c::MOI.ConstraintIndex{MOI.ScalarAffineFunction{Float64},<:_SCALAR_SETS}, ) key = _ConstraintKey(c.value) - if haskey(model.affine_constraint_info, key) - return model.affine_constraint_info[key] + info = get(model.affine_constraint_info, key, nothing) + if info === nothing + return throw(MOI.InvalidIndex(c)) end - return throw(MOI.InvalidIndex(c)) + return info end function row( @@ -1742,14 +1750,13 @@ function MOI.get(model::Optimizer, ::Type{MOI.ConstraintIndex}, name::String) if model.name_to_constraint_index === nothing _rebuild_name_to_constraint_index(model) end - if haskey(model.name_to_constraint_index, name) - constr = model.name_to_constraint_index[name] - if constr === nothing - error("Duplicate constraint name detected: $(name)") - end - return constr + constraint = get(model.name_to_constraint_index, name, missing) + if constraint === nothing + error("Duplicate constraint name detected: $(name)") + elseif constraint === missing + return nothing # No constraint exists with this name end - return nothing + return constraint end function MOI.get( @@ -2545,8 +2552,11 @@ function MOI.is_valid( model::Optimizer, c::MOI.ConstraintIndex{MOI.VariableIndex,S}, ) where {S<:Union{MOI.Semicontinuous{Float64},MOI.Semiinteger{Float64}}} - return haskey(model.variable_info, MOI.VariableIndex(c.value)) && - _info(model, c).bound == _bound_type(S) + info = get(model.variable_info, MOI.VariableIndex(c.value), nothing) + if info === nothing + return false + end + return info.bound == _bound_type(S) end function MOI.add_constraint( From 5cc6fbc5704084324fb3462b6aa84a00a2aa2c45 Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 1 Nov 2024 11:24:20 +1300 Subject: [PATCH 2/5] Add tests --- test/MOI_wrapper.jl | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/MOI_wrapper.jl b/test/MOI_wrapper.jl index 7c04e45..273f323 100644 --- a/test/MOI_wrapper.jl +++ b/test/MOI_wrapper.jl @@ -699,9 +699,20 @@ end function test_is_valid_variable_bound() model = HiGHS.Optimizer() - ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.LessThan{Float64}}(1) - @test_throws MOI.InvalidIndex HiGHS.column(model, ci) - @test !MOI.is_valid(model, ci) + for S in ( + MOI.LessThan{Float64}, + MOI.GreaterThan{Float64}, + MOI.EqualTo{Float64}, + MOI.Interval{Float64}, + MOI.Semicontinuous{Float64}, + MOI.Semiinteger{Float64}, + MOI.ZeroOne, + MOI.Integer + ) + ci = MOI.ConstraintIndex{MOI.VariableIndex,S}(1) + @test_throws MOI.InvalidIndex HiGHS.column(model, ci) + @test !MOI.is_valid(model, ci) + end return end From a88fabfde4ceab676bcf488d84b83e31f05a895f Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Fri, 1 Nov 2024 11:26:30 +1300 Subject: [PATCH 3/5] Update test/MOI_wrapper.jl --- test/MOI_wrapper.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/MOI_wrapper.jl b/test/MOI_wrapper.jl index 273f323..b336159 100644 --- a/test/MOI_wrapper.jl +++ b/test/MOI_wrapper.jl @@ -707,7 +707,7 @@ function test_is_valid_variable_bound() MOI.Semicontinuous{Float64}, MOI.Semiinteger{Float64}, MOI.ZeroOne, - MOI.Integer + MOI.Integer, ) ci = MOI.ConstraintIndex{MOI.VariableIndex,S}(1) @test_throws MOI.InvalidIndex HiGHS.column(model, ci) From 8be2bd2e070c3e267f08db119b4364064fad48bb Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 1 Nov 2024 13:19:44 +1300 Subject: [PATCH 4/5] Update --- src/MOI_wrapper.jl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl index 3a34ae4..47b7319 100644 --- a/src/MOI_wrapper.jl +++ b/src/MOI_wrapper.jl @@ -2428,14 +2428,22 @@ function MOI.is_valid( model::Optimizer, ci::MOI.ConstraintIndex{MOI.VariableIndex,MOI.Integer}, ) - return _info(model, ci).type == _TYPE_INTEGER + info = get(model.variable_info, MOI.VariableIndex(ci.value), nothing) + if info === nothing + return false + end + return info.type == _TYPE_INTEGER end function MOI.is_valid( model::Optimizer, ci::MOI.ConstraintIndex{MOI.VariableIndex,MOI.ZeroOne}, ) - return _info(model, ci).type == _TYPE_BINARY + info = get(model.variable_info, MOI.VariableIndex(ci.value), nothing) + if info === type + return false + end + return info.bound == _TYPE_BINARY end function MOI.get( From ea220b82f2855badfe48ae1fe5e0ae8157bcbfbf Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Fri, 1 Nov 2024 14:26:07 +1300 Subject: [PATCH 5/5] Apply suggestions from code review --- src/MOI_wrapper.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/MOI_wrapper.jl b/src/MOI_wrapper.jl index 47b7319..061b5f1 100644 --- a/src/MOI_wrapper.jl +++ b/src/MOI_wrapper.jl @@ -2440,10 +2440,10 @@ function MOI.is_valid( ci::MOI.ConstraintIndex{MOI.VariableIndex,MOI.ZeroOne}, ) info = get(model.variable_info, MOI.VariableIndex(ci.value), nothing) - if info === type + if info === nothing return false end - return info.bound == _TYPE_BINARY + return info.type == _TYPE_BINARY end function MOI.get(