From e7f885c2fa61fa4919371c2540aa3f9dfd366a08 Mon Sep 17 00:00:00 2001 From: odow Date: Tue, 30 Jan 2024 12:11:31 +1300 Subject: [PATCH 1/3] [Bridges] throw better error when variable cannot be unbridged --- src/Bridges/Variable/map.jl | 24 ++++++++++++++++++++---- src/Bridges/bridge_optimizer.jl | 10 +++++++++- test/Bridges/Variable/map.jl | 2 +- test/Bridges/Variable/zeros.jl | 2 +- test/Bridges/bridge_optimizer.jl | 11 +++++++++++ 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/Bridges/Variable/map.jl b/src/Bridges/Variable/map.jl index cfe16733b3..b3e47de1cd 100644 --- a/src/Bridges/Variable/map.jl +++ b/src/Bridges/Variable/map.jl @@ -572,6 +572,21 @@ function function_for(map::Map, ci::MOI.ConstraintIndex{MOI.VectorOfVariables}) return MOI.VectorOfVariables(variables) end +""" + struct CannotUnbridgeVariableFunction(msg::String) <: Exception + +The error thrown when Variable bridges do not support unmapping the function. + +This is usually because the forward mapping is lossy, for example, when the +`ZerosBridge` converts `x in Zeros()` to `0.0`; given the constant `0.0`, we +cannot recover the variable `x`. +""" +struct CannotUnbridgeVariableFunction <: Exception + msg::String + + CannotUnbridgeVariableFunction(msg::String = "") = new(msg) +end + """ throw_if_cannot_unbridge(map::Map) @@ -579,11 +594,12 @@ Throw an error if some bridged variables do not have any reverse mapping. """ function throw_if_cannot_unbridge(map::Map) if map.unbridged_function === nothing - error( - "Cannot unbridge function because some variables are bridged by", - " variable bridges that do not support reverse mapping, for example,", - " `ZerosBridge`.", + err = CannotUnbridgeVariableFunction( + "Cannot unbridge function because some variables are bridged by " * + "variable bridges that do not support reverse mapping, for " * + "example, `ZerosBridge`.", ) + throw(err) end end diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index a5b5a363b0..976c07baec 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -2381,7 +2381,15 @@ function unbridged_constraint_function( return func end # Otherwise, first unbridge the function: - f = unbridged_function(b, func)::typeof(func) + f = try + unbridged_function(b, func)::typeof(func) + catch err + if err isa Variable.CannotUnbridgeVariableFunction + attr = MOI.ConstraintFunction() + throw(MOI.GetAttributeNotAllowed(attr, err.msg)) + end + rethrow(err) + end # But now we have to deal with an issue. Something like x in [1, ∞) might # get bridged into y in R₊, with x => y + 1, so if the original constraint is # 2x >= 1, the bridged function is 2y >= -1. Unbridging this with y = x - 1 diff --git a/test/Bridges/Variable/map.jl b/test/Bridges/Variable/map.jl index d519273ad0..cf98912ffd 100644 --- a/test/Bridges/Variable/map.jl +++ b/test/Bridges/Variable/map.jl @@ -54,7 +54,7 @@ function test_map() S1 = typeof(set1) v1, c1 = MOI.Bridges.Variable.add_key_for_bridge(map, () -> b1, set1) MOI.is_valid(map, c1) - cannot_unbridge_err = ErrorException( + cannot_unbridge_err = MOI.Bridges.Variable.CannotUnbridgeVariableFunction( "Cannot unbridge function because some variables are bridged by variable" * " bridges that do not support reverse mapping, for example, `ZerosBridge`.", ) diff --git a/test/Bridges/Variable/zeros.jl b/test/Bridges/Variable/zeros.jl index 73cf9d779a..1c7ce1ec50 100644 --- a/test/Bridges/Variable/zeros.jl +++ b/test/Bridges/Variable/zeros.jl @@ -84,7 +84,7 @@ function test_zeros() ) @test_throws err MOI.delete(bridged_mock, cyz) - err = ErrorException( + err = MOI.Bridges.Variable.CannotUnbridgeVariableFunction( "Cannot unbridge function because some variables are bridged by" * " variable bridges that do not support reverse mapping, for example," * " `ZerosBridge`.", diff --git a/test/Bridges/bridge_optimizer.jl b/test/Bridges/bridge_optimizer.jl index 9493f4c0e4..97fe163f03 100644 --- a/test/Bridges/bridge_optimizer.jl +++ b/test/Bridges/bridge_optimizer.jl @@ -1176,6 +1176,17 @@ function test_ListOfVariablesWithAttributeSet(T = Float64) return end +function test_cannot_unbridge_variable_function() + model = MOI.Bridges.Variable.Zeros{Float64}(MOI.Utilities.Model{Float64}()) + x, c = MOI.add_constrained_variables(model, MOI.Zeros(1)) + c2 = MOI.add_constraint(model, 1.0 * x[1], MOI.EqualTo(1.0)) + @test_throws( + MOI.GetAttributeNotAllowed{MOI.ConstraintFunction}, + MOI.get(model, MOI.ConstraintFunction(), c2), + ) + return +end + end # module TestBridgeOptimizer.runtests() From 8f4099e5a58877338a4762adcf0721463f263012 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Wed, 31 Jan 2024 11:58:17 +1300 Subject: [PATCH 2/3] Update zeros.jl --- test/Bridges/Variable/zeros.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/Bridges/Variable/zeros.jl b/test/Bridges/Variable/zeros.jl index 1c7ce1ec50..3e26052cd4 100644 --- a/test/Bridges/Variable/zeros.jl +++ b/test/Bridges/Variable/zeros.jl @@ -91,7 +91,13 @@ function test_zeros() ) @test_throws err MOI.get(bridged_mock, MOI.ObjectiveFunction{typeof(obj)}()) # With `c1`, the function does not contain any variable so it tests that it - # also throws an error even if it never calls `variable_unbridged_function`. + # also throws an error even if it never calls `variable_unbridged_function` + err = MOI.GetAttributeNotAllowed( + MOI.ConstraintFunction(), + "Cannot unbridge function because some variables are bridged by" * + " variable bridges that do not support reverse mapping, for example," * + " `ZerosBridge`.", + ) @test_throws err MOI.get(bridged_mock, MOI.ConstraintFunction(), c1) @test_throws err MOI.get(bridged_mock, MOI.ConstraintFunction(), c2) From ddebdf8f3052a78bf6807252ed83f87778a4c8aa Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 9 Feb 2024 09:19:33 +1300 Subject: [PATCH 3/3] Throw GetAttributeNotAllowed --- src/Bridges/Variable/map.jl | 18 ++---------------- src/Bridges/bridge_optimizer.jl | 10 +--------- test/Bridges/Variable/map.jl | 3 ++- test/Bridges/Variable/zeros.jl | 9 ++------- 4 files changed, 7 insertions(+), 33 deletions(-) diff --git a/src/Bridges/Variable/map.jl b/src/Bridges/Variable/map.jl index b3e47de1cd..eca4290a41 100644 --- a/src/Bridges/Variable/map.jl +++ b/src/Bridges/Variable/map.jl @@ -572,21 +572,6 @@ function function_for(map::Map, ci::MOI.ConstraintIndex{MOI.VectorOfVariables}) return MOI.VectorOfVariables(variables) end -""" - struct CannotUnbridgeVariableFunction(msg::String) <: Exception - -The error thrown when Variable bridges do not support unmapping the function. - -This is usually because the forward mapping is lossy, for example, when the -`ZerosBridge` converts `x in Zeros()` to `0.0`; given the constant `0.0`, we -cannot recover the variable `x`. -""" -struct CannotUnbridgeVariableFunction <: Exception - msg::String - - CannotUnbridgeVariableFunction(msg::String = "") = new(msg) -end - """ throw_if_cannot_unbridge(map::Map) @@ -594,7 +579,8 @@ Throw an error if some bridged variables do not have any reverse mapping. """ function throw_if_cannot_unbridge(map::Map) if map.unbridged_function === nothing - err = CannotUnbridgeVariableFunction( + err = MOI.GetAttributeNotAllowed( + MOI.ConstraintFunction(), "Cannot unbridge function because some variables are bridged by " * "variable bridges that do not support reverse mapping, for " * "example, `ZerosBridge`.", diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 976c07baec..a5b5a363b0 100644 --- a/src/Bridges/bridge_optimizer.jl +++ b/src/Bridges/bridge_optimizer.jl @@ -2381,15 +2381,7 @@ function unbridged_constraint_function( return func end # Otherwise, first unbridge the function: - f = try - unbridged_function(b, func)::typeof(func) - catch err - if err isa Variable.CannotUnbridgeVariableFunction - attr = MOI.ConstraintFunction() - throw(MOI.GetAttributeNotAllowed(attr, err.msg)) - end - rethrow(err) - end + f = unbridged_function(b, func)::typeof(func) # But now we have to deal with an issue. Something like x in [1, ∞) might # get bridged into y in R₊, with x => y + 1, so if the original constraint is # 2x >= 1, the bridged function is 2y >= -1. Unbridging this with y = x - 1 diff --git a/test/Bridges/Variable/map.jl b/test/Bridges/Variable/map.jl index cf98912ffd..932b01d912 100644 --- a/test/Bridges/Variable/map.jl +++ b/test/Bridges/Variable/map.jl @@ -54,7 +54,8 @@ function test_map() S1 = typeof(set1) v1, c1 = MOI.Bridges.Variable.add_key_for_bridge(map, () -> b1, set1) MOI.is_valid(map, c1) - cannot_unbridge_err = MOI.Bridges.Variable.CannotUnbridgeVariableFunction( + cannot_unbridge_err = MOI.GetAttributeNotAllowed( + MOI.ConstraintFunction(), "Cannot unbridge function because some variables are bridged by variable" * " bridges that do not support reverse mapping, for example, `ZerosBridge`.", ) diff --git a/test/Bridges/Variable/zeros.jl b/test/Bridges/Variable/zeros.jl index 3e26052cd4..923bc868b0 100644 --- a/test/Bridges/Variable/zeros.jl +++ b/test/Bridges/Variable/zeros.jl @@ -84,7 +84,8 @@ function test_zeros() ) @test_throws err MOI.delete(bridged_mock, cyz) - err = MOI.Bridges.Variable.CannotUnbridgeVariableFunction( + err = MOI.GetAttributeNotAllowed( + MOI.ConstraintFunction(), "Cannot unbridge function because some variables are bridged by" * " variable bridges that do not support reverse mapping, for example," * " `ZerosBridge`.", @@ -92,12 +93,6 @@ function test_zeros() @test_throws err MOI.get(bridged_mock, MOI.ObjectiveFunction{typeof(obj)}()) # With `c1`, the function does not contain any variable so it tests that it # also throws an error even if it never calls `variable_unbridged_function` - err = MOI.GetAttributeNotAllowed( - MOI.ConstraintFunction(), - "Cannot unbridge function because some variables are bridged by" * - " variable bridges that do not support reverse mapping, for example," * - " `ZerosBridge`.", - ) @test_throws err MOI.get(bridged_mock, MOI.ConstraintFunction(), c1) @test_throws err MOI.get(bridged_mock, MOI.ConstraintFunction(), c2)