From 541c44c818b0678dcac807df6789279158b653c7 Mon Sep 17 00:00:00 2001 From: David Bach Date: Thu, 21 Sep 2023 15:32:06 +0300 Subject: [PATCH 1/7] Truncate large error --- src/SolverAPI.jl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index 12ca729..33a9556 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -522,7 +522,11 @@ function add_obj!( g = canonicalize_SNF(T, json_to_snf(a, vars_map)) g_type = MOI.ObjectiveFunction{typeof(g)}() if !MOI.supports(model, g_type) - throw(Error(Unsupported, "Objective function $g isn't supported by this solver.")) + g_repr = string(g) + if length(g_repr) > 100 + g_repr = g_repr[1:100] * " ... (truncated)" + end + throw(Error(Unsupported, "Objective function $g_repr isn't supported by this solver.")) end MOI.set(model, g_type, g) return nothing From 5c66f1150b67dda9a3ba93688b0f7e3d0880a42d Mon Sep 17 00:00:00 2001 From: David Bach Date: Thu, 21 Sep 2023 15:32:59 +0300 Subject: [PATCH 2/7] Increase cutoff size --- src/SolverAPI.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index 33a9556..144d384 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -523,8 +523,8 @@ function add_obj!( g_type = MOI.ObjectiveFunction{typeof(g)}() if !MOI.supports(model, g_type) g_repr = string(g) - if length(g_repr) > 100 - g_repr = g_repr[1:100] * " ... (truncated)" + if length(g_repr) > 256 + g_repr = g_repr[1:256] * " ... (truncated)" end throw(Error(Unsupported, "Objective function $g_repr isn't supported by this solver.")) end From 3d46d6d6021cb71ab7f52b7388b22b4141239c07 Mon Sep 17 00:00:00 2001 From: David Bach Date: Thu, 21 Sep 2023 15:59:34 +0300 Subject: [PATCH 3/7] JuliaFormatter --- src/SolverAPI.jl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index 144d384..52c6267 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -525,8 +525,13 @@ function add_obj!( g_repr = string(g) if length(g_repr) > 256 g_repr = g_repr[1:256] * " ... (truncated)" - end - throw(Error(Unsupported, "Objective function $g_repr isn't supported by this solver.")) + end + throw( + Error( + Unsupported, + "Objective function $g_repr isn't supported by this solver.", + ), + ) end MOI.set(model, g_type, g) return nothing From b635836683e857b14dd49d1d20db9c875354839b Mon Sep 17 00:00:00 2001 From: Chris Coey Date: Thu, 21 Sep 2023 13:00:17 -0700 Subject: [PATCH 4/7] refactor --- CHANGELOG.md | 10 ---------- src/SolverAPI.jl | 25 +++++++++++++------------ 2 files changed, 13 insertions(+), 22 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index 8f8a2cc..0000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,10 +0,0 @@ -# Changelog - -All notable changes to this project will be documented in this file. - -The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), -and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - -## [0.1.0] - - - Initial release diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index 52c6267..ca4740f 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -487,7 +487,7 @@ function nl_to_aff_or_quad(::Type{T}, f::MOI.ScalarNonlinearFunction) where {T<: isnothing(h) || return MOI.Utilities.operate(h, T, args...) end end - throw(Error(Domain, "Function $f cannot be converted to linear or quadratic form.")) + return error() # Gets caught by canonicalize_SNF. end nl_to_aff_or_quad(::Type{<:Real}, f::MOI.VariableIndex) = f @@ -522,16 +522,8 @@ function add_obj!( g = canonicalize_SNF(T, json_to_snf(a, vars_map)) g_type = MOI.ObjectiveFunction{typeof(g)}() if !MOI.supports(model, g_type) - g_repr = string(g) - if length(g_repr) > 256 - g_repr = g_repr[1:256] * " ... (truncated)" - end - throw( - Error( - Unsupported, - "Objective function $g_repr isn't supported by this solver.", - ), - ) + msg = "Objective function $(trunc_str(g)) isn't supported by this solver." + throw(Error(Unsupported, msg)) end MOI.set(model, g_type, g) return nothing @@ -635,7 +627,7 @@ function add_cons!( g = shift_terms(T, f.args) s = S(zero(T)) if !MOI.supports_constraint(model, typeof(g), typeof(s)) - msg = "Constraint $g in $s isn't supported by this solver." + msg = "Constraint $(trunc_str(g)) in $(trunc_str(s)) isn't supported by this solver." throw(Error(Unsupported, msg)) end ci = MOI.Utilities.normalize_and_add_constraint(model, g, s) @@ -653,4 +645,13 @@ function shift_terms(::Type{T}, args::Vector) where {T<:Real} return MOI.Utilities.operate(-, T, g1, g2) end +# Convert object to string and truncate string length if too long. +function trunc_str(f::Union{MOI.AbstractScalarFunction,MOI.AbstractScalarSet}) + f_str = string(f) + if length(f_str) > 256 + f_str = f_str[1:256] * " ... (truncated)" + end + return f_str +end + end # module SolverAPI From a2b7388a095b888a99f448ef4260ffa2e40b5f47 Mon Sep 17 00:00:00 2001 From: Chris Coey Date: Thu, 21 Sep 2023 14:56:39 -0700 Subject: [PATCH 5/7] fix error handling so we always return a response of errors rather than throwing; add tests --- src/SolverAPI.jl | 47 +++++-------- test/all_tests.jl | 74 ++++++++++++++------- test/inputs/feas_range.json | 2 +- test/inputs/incorrect_range_num_params.json | 2 +- test/inputs/incorrect_range_step_not_1.json | 2 +- test/inputs/unsupported_con_sign.json | 1 + test/inputs/unsupported_con_type.json | 1 + test/inputs/unsupported_obj_type.json | 1 + test/inputs/unsupported_operator.json | 1 + test/inputs/unsupported_solver_option.json | 1 + 10 files changed, 75 insertions(+), 57 deletions(-) create mode 100644 test/inputs/unsupported_con_sign.json create mode 100644 test/inputs/unsupported_con_type.json create mode 100644 test/inputs/unsupported_obj_type.json create mode 100644 test/inputs/unsupported_operator.json create mode 100644 test/inputs/unsupported_solver_option.json diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index ca4740f..a61d955 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -176,9 +176,7 @@ gracefully and included in `Response`. """ function solve(fn, json::Request, solver::MOI.AbstractOptimizer) errors = validate(json) - if length(errors) > 0 - return response(; errors) - end + isempty(errors) || return response(; errors) try T, solver_info, model = initialize(json, solver) @@ -190,10 +188,11 @@ function solve(fn, json::Request, solver::MOI.AbstractOptimizer) end return response(json, model, solver) catch e + _err(E) = response(Error(E, sprint(Base.showerror, e))) if e isa MOI.UnsupportedError - throw(Error(Unsupported, sprint(Base.showerror, e))) + return _err(Unsupported) elseif e isa MOI.NotAllowedError - throw(Error(NotAllowed, sprint(Base.showerror, e))) + return _err(NotAllowed) elseif e isa MOI.InvalidIndex || e isa MOI.ResultIndexBoundsError || e isa MOI.ScalarFunctionConstantNotZero || @@ -201,11 +200,11 @@ function solve(fn, json::Request, solver::MOI.AbstractOptimizer) e isa MOI.UpperBoundAlreadySet || e isa MOI.OptimizeInProgress || e isa MOI.InvalidCallbackUsage - throw(Error(Domain, sprint(Base.showerror, e))) + return _err(Domain) elseif e isa ErrorException - throw(Error(Other, e.msg)) + return _err(Other) else - rethrow() + return response(e) end end end @@ -353,16 +352,6 @@ function validate(json::Request)#::Vector{Error} end end - for con in json.constraints - if first(con) == "range" - if length(con) != 5 - _err("The `range` constraint expects 4 arguments.") - elseif con[4] != 1 - _err("The `range` constraint expects a step size of 1.") - end - end - end - return out end @@ -537,14 +526,6 @@ function add_cons!( solver_info::Dict, ) where {T<:Real} head = a[1] - - function _check_v_type(v) - if !(v isa MOI.VariableIndex) - msg = "Variable $v must be of type MOI.VariableIndex, not $(typeof(v))." - throw(Error(InvalidModel, msg)) - end - end - if head == "and" for i in eachindex(a) i == 1 && continue @@ -577,13 +558,17 @@ function add_cons!( f = MOI.ScalarNonlinearFunction(:abs, Any[v]) MOI.add_constraint(model, f, MOI.EqualTo(1)) elseif head == "range" + if length(a) != 5 + throw(Error(InvalidModel, "The `range` constraint expects 4 arguments.")) + end v = json_to_snf(a[5], vars_map) - + _check_v_type(v) if !(a[2] isa Int && a[3] isa Int) throw(Error(InvalidModel, "The `range` constraint expects integer bounds.")) end - _check_v_type(v) - + if a[4] != 1 + throw(Error(InvalidModel, "The `range` constraint expects a step size of 1.")) + end MOI.add_constraint(model, v, MOI.Integer()) MOI.add_constraint(model, v, MOI.Interval{T}(a[2], a[3])) elseif head == "implies" && solver_info[:use_indicator] @@ -636,6 +621,10 @@ function add_cons!( return nothing end +_check_v_type(::MOI.VariableIndex) = nothing +_check_v_type(_) = + throw(Error(InvalidModel, "$v must be a `MOI.VariableIndex`, not $(typeof(v)).")) + ineq_to_moi = Dict(:<= => MOI.LessThan, :>= => MOI.GreaterThan, :(==) => MOI.EqualTo) function shift_terms(::Type{T}, args::Vector) where {T<:Real} diff --git a/test/all_tests.jl b/test/all_tests.jl index 5461b1f..41d2fc2 100644 --- a/test/all_tests.jl +++ b/test/all_tests.jl @@ -75,35 +75,59 @@ end end end -@testitem "validate" setup = [SolverSetup] begin - using SolverAPI: deserialize, validate +@testitem "errors" setup = [SolverSetup] begin + using SolverAPI import JSON3 # scenarios with incorrect format - format_err_json_names = [ - # TODO fix: error not thrown for "unsupported_print_format" - # "unsupported_print_format", # print format not supported - "feas_with_obj", # objective provided for a feasibility problem - "min_no_obj", # no objective function specified for a minimization problem - "unsupported_sense", # unsupported sense such as 'feasiblity' - "obj_len_greater_than_1", # length of objective greater than 1 - "incorrect_range_num_params", # number of parameters not equal to 4 - "incorrect_range_step_not_1", # step not one in range definition - "vars_is_not_str", # field variables is not a string - "vars_is_not_arr", # field variables is not an array - "objs_is_not_arr", # field objectives is not an array - "cons_is_not_arr", # field constraints is not an array - "missing_vars", # missing field variables - "missing_cons", # missing field constraints - "missing_objs", # missing field objectives - "missing_sense", # missing field sense - "missing_version", # missing field version + json_names_and_errors = [ + # missing field variables + ("missing_vars", "InvalidFormat"), + # missing field constraints + ("missing_cons", "InvalidFormat"), + # missing field objectives + ("missing_objs", "InvalidFormat"), + # missing field sense + ("missing_sense", "InvalidFormat"), + # missing field version + ("missing_version", "InvalidFormat"), + # field variables is not a string + ("vars_is_not_str", "InvalidFormat"), + # field variables is not an array + ("vars_is_not_arr", "InvalidFormat"), + # field objectives is not an array + ("objs_is_not_arr", "InvalidFormat"), + # field constraints is not an array + ("cons_is_not_arr", "InvalidFormat"), + # length of objective greater than 1 + ("obj_len_greater_than_1", "InvalidFormat"), + # objective provided for a feasibility problem + ("feas_with_obj", "InvalidFormat"), + # no objective function specified for a minimization problem + ("min_no_obj", "InvalidFormat"), + # unsupported sense such as 'feasibility' + ("unsupported_sense", "InvalidFormat"), + # range: wrong number of args + ("incorrect_range_num_params", "InvalidModel"), + # range: step not one + ("incorrect_range_step_not_1", "InvalidModel"), + # unsupported objective function type + ("unsupported_obj_type", "Unsupported"), + # unsupported constraint function type + ("unsupported_con_type", "Unsupported"), + # unsupported constraint sign + ("unsupported_con_sign", "Unsupported"), + # unsupported operator + ("unsupported_operator", "Unsupported"), + # unsupported solver option + ("unsupported_solver_option", "Unsupported"), + # print format not supported + ("unsupported_print_format", "Unsupported"), ] - @testset "$j" for j in format_err_json_names - input = deserialize(read_json("inputs", j)) - errors = validate(input) - @test errors isa Vector{SolverAPI.Error} - @test length(errors) >= 1 + @testset "$j" for (j, es...) in json_names_and_errors + result = JSON3.read(run_solve(read_json("inputs", j))) + @test haskey(result, :errors) && length(result.errors) >= 1 + @test Set(e.type for e in result.errors) == Set(es) end end diff --git a/test/inputs/feas_range.json b/test/inputs/feas_range.json index fb5b943..d4f7b85 100644 --- a/test/inputs/feas_range.json +++ b/test/inputs/feas_range.json @@ -1 +1 @@ -{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range",9, 9, 1, "x"],["Float","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} +{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range",9,9,1,"x"],["Float","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} diff --git a/test/inputs/incorrect_range_num_params.json b/test/inputs/incorrect_range_num_params.json index 0d7bf99..55d60d7 100644 --- a/test/inputs/incorrect_range_num_params.json +++ b/test/inputs/incorrect_range_num_params.json @@ -1 +1 @@ -{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range", 9, 9, "x"],["Int","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} +{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range",9,9,"x"],["Int","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} diff --git a/test/inputs/incorrect_range_step_not_1.json b/test/inputs/incorrect_range_step_not_1.json index 88dbf36..b7146b2 100644 --- a/test/inputs/incorrect_range_step_not_1.json +++ b/test/inputs/incorrect_range_step_not_1.json @@ -1 +1 @@ -{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range",9, 9, 2, "x"],["Int","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} +{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["range",9,9,2,"x"],["Int","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} diff --git a/test/inputs/unsupported_con_sign.json b/test/inputs/unsupported_con_sign.json new file mode 100644 index 0000000..e079ddc --- /dev/null +++ b/test/inputs/unsupported_con_sign.json @@ -0,0 +1 @@ +{"version":"0.1","sense":"min","variables":["x","y"],"constraints":[["and",["!=",["+","x",["*",3,"y"]],1],[">=",["+","x","y"],1]],["and",["Int","x"],["Nonneg","x"]],["Int","y"]],"objectives":[["+",["*",2,"x"],"y"]],"options":{"solver":"HiGHS"}} diff --git a/test/inputs/unsupported_con_type.json b/test/inputs/unsupported_con_type.json new file mode 100644 index 0000000..ea6dd3a --- /dev/null +++ b/test/inputs/unsupported_con_type.json @@ -0,0 +1 @@ +{"version":"0.1","sense":"min","variables":["x","y"],"constraints":[["and",["==",["*","x",["*","x","y"]],1],[">=",["+","x","y"],1]],["and",["Int","x"],["Nonneg","x"]],["Int","y"]],"objectives":[["+",["*",2,"x"],"y"]],"options":{"solver":"HiGHS"}} diff --git a/test/inputs/unsupported_obj_type.json b/test/inputs/unsupported_obj_type.json new file mode 100644 index 0000000..8b07678 --- /dev/null +++ b/test/inputs/unsupported_obj_type.json @@ -0,0 +1 @@ +{"version":"0.1","sense":"min","variables":["x","y"],"constraints":[["and",["==",["+","x",["*",3,"y"]],1],[">=",["+","x","y"],1]],["and",["Int","x"],["Nonneg","x"]],["Int","y"]],"objectives":[["/",["*",2,"x"],"y"]],"options":{"solver":"HiGHS"}} diff --git a/test/inputs/unsupported_operator.json b/test/inputs/unsupported_operator.json new file mode 100644 index 0000000..0b72755 --- /dev/null +++ b/test/inputs/unsupported_operator.json @@ -0,0 +1 @@ +{"version":"0.1","sense":"feas","variables":["x"],"constraints":[["fake_operator","x",1],["Int","x"]],"objectives":[],"options":{"solver":"MiniZinc"}} diff --git a/test/inputs/unsupported_solver_option.json b/test/inputs/unsupported_solver_option.json new file mode 100644 index 0000000..fdf66a3 --- /dev/null +++ b/test/inputs/unsupported_solver_option.json @@ -0,0 +1 @@ +{"version":"0.1","sense":"min","variables":["x"],"constraints":[["==","x",1],["Int","x"]],"objectives":["x"],"options":{"time_limit_sec":60,"solver":"HiGHS","fake_option":0}} From 590ecd3f2a6af981103f18905bb934527213145d Mon Sep 17 00:00:00 2001 From: David Bach Date: Fri, 22 Sep 2023 14:00:33 +0300 Subject: [PATCH 6/7] Add CHANGELOG back --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..8f8a2cc --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [0.1.0] + + - Initial release From 1c02f99c10c5127ddd4e0252d07921fc857d8014 Mon Sep 17 00:00:00 2001 From: David Bach Date: Fri, 22 Sep 2023 14:08:28 +0300 Subject: [PATCH 7/7] Rethrow unknown errors --- src/SolverAPI.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SolverAPI.jl b/src/SolverAPI.jl index a61d955..06d0269 100644 --- a/src/SolverAPI.jl +++ b/src/SolverAPI.jl @@ -203,8 +203,10 @@ function solve(fn, json::Request, solver::MOI.AbstractOptimizer) return _err(Domain) elseif e isa ErrorException return _err(Other) - else + elseif e isa Error return response(e) + else + rethrow() end end end