fix tests

Author: BatyLeo

src/FixedSizeShortestPath/FixedSizeShortestPath.jl

@@ -54,6 +54,12 @@ function FixedSizeShortestPathBenchmark(;
     return FixedSizeShortestPathBenchmark(g, grid_size, p, deg, ν)
 end
 
+function Utils.objective_value(
+    ::FixedSizeShortestPathBenchmark, θ::AbstractArray, y::AbstractArray
+)
+    return -dot(θ, y)
+end
+
 """
 $TYPEDSIGNATURES
 

src/StochasticVehicleScheduling/StochasticVehicleScheduling.jl

@@ -26,15 +26,7 @@ using Graphs:
     inneighbors,
     outneighbors
 using JuMP:
-    Model,
-    @variable,
-    @objective,
-    @constraint,
-    optimize!,
-    value,
-    objective_value,
-    set_silent,
-    dual
+    JuMP, Model, @variable, @objective, @constraint, optimize!, value, set_silent, dual
 using Plots: Plots, plot, plot!, scatter!, annotate!, text
 using Printf: @printf
 using Random: Random, AbstractRNG, MersenneTwister
@@ -81,7 +73,7 @@ end
 $TYPEDSIGNATURES
 
 Create a dataset of `dataset_size` instances for the given `StochasticVehicleSchedulingBenchmark`.
-If you want to also add label solutions in the dataset, set `compute_solutions=true`.
+If you want to not add label solutions in the dataset, set `compute_solutions=false`.
 By default, they will be computed using column generation.
 Note that computing solutions can be time-consuming, especially for large instances.
 You can also use instead `compact_mip` or `compact_linearized_mip` as the algorithm to compute solutions.
@@ -92,7 +84,7 @@ If `store_city=false`, the coordinates and unnecessary information about instanc
 function Utils.generate_dataset(
     benchmark::StochasticVehicleSchedulingBenchmark,
     dataset_size::Int;
-    compute_solutions=false,
+    compute_solutions=true,
     seed=nothing,
     rng=MersenneTwister(0),
     algorithm=column_generation_algorithm,
@@ -133,6 +125,9 @@ function Utils.generate_statistical_model(bench::StochasticVehicleSchedulingBenc
     return Chain(Dense(20 => 1; bias=false), vec)
 end
 
+"""
+$TYPEDSIGNATURES
+"""
 function plot_instance(
     ::StochasticVehicleSchedulingBenchmark,
     sample::DataSample{<:Instance{City}};
@@ -193,6 +188,9 @@ function plot_instance(
     return fig
 end
 
+"""
+$TYPEDSIGNATURES
+"""
 function plot_solution(
     ::StochasticVehicleSchedulingBenchmark, sample::DataSample{<:Instance{City}}; kwargs...
 )

src/StochasticVehicleScheduling/solution/algorithms/column_generation.jl

@@ -87,9 +87,9 @@ function column_generation(
         )
     end
 
-    c_low = Float64(objective_value(model))
+    c_low = JuMP.objective_value(model)
     columns = unique(cat(initial_paths, new_paths; dims=1))
-    return columns, c_low, value.(λ)
+    return columns, c_low::Float64, value.(λ)
 end
 
 """
@@ -140,7 +140,7 @@ function compute_solution_from_selected_columns(
     optimize!(model)
 
     sol = value.(y)
-    return Float64(objective_value(model)),
+    return JuMP.objective_value(model)::Float64,
     sol,
     paths[isapprox.([sol[p] for p in paths], 1.0)]
 end

src/StochasticVehicleScheduling/solution/algorithms/local_search.jl

@@ -49,7 +49,7 @@ function solve_deterministic_VSP(
 
     solution = solution_from_JuMP_array(value.(x), graph)
 
-    return objective_value(model), solution
+    return JuMP.objective_value(model), solution
 end
 
 """
@@ -74,7 +74,9 @@ function move_one_random_task!(path_value::BitMatrix, graph::AbstractGraph)
         if selected_task > 1
             before = @view path_value[i, 1:(selected_task - 1)]
             if any(before)
-                precedent_task = selected_task - find_first_one(reverse(before))
+                aaa = find_first_one(reverse(before))
+                @assert aaa >= 0
+                precedent_task = selected_task - aaa
                 if !has_edge(graph, precedent_task + 1, selected_task + 1)
                     continue
                 end
@@ -88,9 +90,10 @@ function move_one_random_task!(path_value::BitMatrix, graph::AbstractGraph)
         if selected_task < nb_tasks
             after = @view path_value[i, (selected_task + 1):end]
             if any(after)
-                next_task =
-                    selected_task +
-                    find_first_one(@view path_value[i, (selected_task + 1):end])
+                bbb = find_first_one(@view path_value[i, (selected_task + 1):end])
+                @assert bbb >= 0
+                next_task = selected_task + bbb
+
                 if !has_edge(graph, selected_task + 1, next_task + 1)
                     continue
                 end
@@ -138,7 +141,6 @@ function _local_search(solution::Solution, instance::Instance; nb_it::Integer=10
             candidate_solution = copy(best_solution)
         end
     end
-
     return Solution(best_solution, instance), best_value, history_x, history_y
 end
 

src/StochasticVehicleScheduling/solution/solution.jl

@@ -15,32 +15,32 @@ struct Solution
     path_value::BitMatrix  # list of vehicles paths
 end
 
-function get_nb_vehicles(path_value::BitMatrix)
-    return sum(any(path_value; dims=2))
-end
-
-function get_nb_vehicles(solution::Solution)
-    return get_nb_vehicles(solution.path_value)
-end
-
-"""
-$TYPEDSIGNATURES
-
-Compute routes of solution.
-"""
-function get_routes(solution::Solution)
-    res = Vector{Int}[]
-    for vehicle in 1:get_nb_vehicles(solution)
-        resv = Int[]
-        for (index, value) in enumerate(solution.path_value[vehicle, :])
-            if value
-                push!(resv, index + 1)
-            end
-        end
-        push!(res, resv)
-    end
-    return res
-end
+# function get_nb_vehicles(path_value::BitMatrix)
+#     return sum(any(path_value; dims=2))
+# end
+
+# function get_nb_vehicles(solution::Solution)
+#     return get_nb_vehicles(solution.path_value)
+# end
+
+# """
+# $TYPEDSIGNATURES
+
+# Compute routes of solution.
+# """
+# function get_routes(solution::Solution)
+#     res = Vector{Int}[]
+#     for vehicle in 1:get_nb_vehicles(solution)
+#         resv = Int[]
+#         for (index, value) in enumerate(solution.path_value[vehicle, :])
+#             if value
+#                 push!(resv, index + 1)
+#             end
+#         end
+#         push!(res, resv)
+#     end
+#     return res
+# end
 
 """
 $TYPEDSIGNATURES
@@ -152,32 +152,32 @@ function path_solution_from_JuMP_array(x::AbstractArray, graph::AbstractGraph)
     return sol
 end
 
-function basic_path_solution(graph::AbstractGraph)
-    nb_tasks = nv(graph) - 2
-    sol = falses(nb_tasks, nb_tasks)
-    for i_task in 1:nb_tasks
-        sol[i_task, i_task] = true
-    end
-    return sol
-end
-
-"""
-$TYPEDSIGNATURES
-
-Create a solution with one vehicle per task.
-"""
-function basic_solution(instance::Instance)
-    graph = instance.graph
-    value = falses(ne(graph))
-
-    for (a, edge) in enumerate(edges(graph))
-        if edge.src == 1 || edge.dst == nv(graph)
-            value[a] = true
-        end
-    end
-
-    return Solution(value, basic_path_solution(graph))
-end
+# function basic_path_solution(graph::AbstractGraph)
+#     nb_tasks = nv(graph) - 2
+#     sol = falses(nb_tasks, nb_tasks)
+#     for i_task in 1:nb_tasks
+#         sol[i_task, i_task] = true
+#     end
+#     return sol
+# end
+
+# """
+# $TYPEDSIGNATURES
+
+# Create a solution with one vehicle per task.
+# """
+# function basic_solution(instance::Instance)
+#     graph = instance.graph
+#     value = falses(ne(graph))
+
+#     for (a, edge) in enumerate(edges(graph))
+#         if edge.src == 1 || edge.dst == nv(graph)
+#             value[a] = true
+#         end
+#     end
+
+#     return Solution(value, basic_path_solution(graph))
+# end
 
 """
 $TYPEDSIGNATURES
@@ -290,7 +290,7 @@ function to_array(path_value::BitMatrix, instance::Instance)
         current_task = 1
         while true
             index_shift = find_first_one(@view path_value[i, current_task:end])
-            if isnothing(index_shift)
+            if index_shift === -1
                 mat[current_task, nb_nodes] = true
                 break
             end
@@ -323,7 +323,7 @@ $TYPEDSIGNATURES
 
 Check if `solution` is an admissible solution of `instance`.
 """
-function is_feasible(solution::Solution, instance::Instance)
+function is_feasible(solution::Solution, instance::Instance; verbose=true)
     graph = instance.graph
     nb_nodes = nv(graph)
     nb_tasks = nb_nodes - 2
@@ -339,14 +339,13 @@ function is_feasible(solution::Solution, instance::Instance)
         current_task = 1
         while true
             index_shift = find_first_one(@view solution.path_value[i, current_task:end])
-            if isnothing(index_shift)
+            if index_shift == -1
                 mat[current_task, nb_nodes] = true
                 break
             end
             next_task = current_task + index_shift
             if !has_edge(graph, current_task, next_task)
-                @warn "Flow not respected" current_task next_task
-                @warn "" outneighbors(graph, current_task)
+                verbose && @warn "Flow not respected" current_task next_task
                 return false
             end
             mat[current_task, next_task] = true
@@ -355,21 +354,15 @@ function is_feasible(solution::Solution, instance::Instance)
     end
 
     if !all(sum(solution.path_value; dims=1) .== 1)
-        @warn "One task done by more than one vehicle (or less than once)"
+        verbose && @warn "One task done by more than one vehicle (or less than once)"
         return false
     end
 
     for i in job_indices
         s1 = sum(mat[j, i] for j in inneighbors(graph, i))
         s2 = sum(mat[i, j] for j in outneighbors(graph, i))
         if s1 != s2 || s1 != 1
-            @warn "Flow is broken" i s1 s2
-            @warn "" inneighbors(graph, i)
-            @warn "" [mat[j, i] for j in inneighbors(graph, i)]
-            @warn "" outneighbors(graph, i)
-            @warn "" [mat[i, j] for j in outneighbors(graph, i)]
-            @warn "" mat
-            @warn "" solution.path_value
+            verbose && @warn "Flow is broken" i s1 s2
             return false
         end
     end
@@ -380,7 +373,7 @@ end
 function compute_path_list(solution::Solution)
     (; path_value) = solution
     paths = Vector{Int64}[]
-    for v in 1:size(path_value, 1)
+    for v in axes(path_value, 1)
         path = [1]
         for (i, elem) in enumerate(path_value[v, :])
             if elem == 1

src/StochasticVehicleScheduling/utils.jl

@@ -46,7 +46,7 @@ function find_first_one(A::AbstractVector)
             return i
         end
     end
-    return nothing
+    return -1
 end
 
 # Config stuff, probably not needed in this package

src/Utils/interface.jl

@@ -122,20 +122,31 @@ end
 """
 $TYPEDSIGNATURES
 
+Check if the benchmark is a minimization problem.
+"""
+function is_minimization_problem(::AbstractBenchmark)
+    return true
+end
+
+"""
+$TYPEDSIGNATURES
+
 Default behaviour of `compute_gap` for a benchmark problem where `features`, `solutions` and `costs` are all defined.
 """
 function compute_gap(
     bench::AbstractBenchmark, dataset::Vector{<:DataSample}, statistical_model, maximizer
 )
     res = 0.0
+    check = is_minimization_problem(bench)
 
     for sample in dataset
         target_obj = objective_value(bench, sample)
         x = sample.x
         θ = statistical_model(x)
         y = maximizer(θ; maximizer_kwargs(bench, sample)...)
         obj = objective_value(bench, sample, y)
-        res += (target_obj - obj) / abs(target_obj)
+        Δ = check ? obj - target_obj : target_obj - obj
+        res += Δ / abs(target_obj)
     end
     return res / length(dataset)
 end

src/Warcraft/Warcraft.jl

@@ -25,6 +25,10 @@ Does not have any field.
 """
 struct WarcraftBenchmark <: AbstractBenchmark end
 
+function Utils.objective_value(::WarcraftBenchmark, θ::AbstractArray, y::AbstractArray)
+    return -dot(θ, y)
+end
+
 """
 $TYPEDSIGNATURES