jump-dev / MathOptInterface.jl

A data structure for mathematical optimization problems
http://jump.dev/MathOptInterface.jl/
Other
392 stars 87 forks source link

Test names clash #2127

Closed blegat closed 1 year ago

blegat commented 1 year ago

When excluding test_conic_PositiveSemidefiniteConeTriangle, CSDP actually also exclude:

which is an issue since it is passing these tests! MOI.Test should error in case the name of one test is included in the name of another test.

odow commented 1 year ago

Ah. I didn't think about this case.

odow commented 1 year ago

Here's the list:

julia> import MathOptInterface as MOI

julia> begin
           tests = filter(n -> startswith("$n", "test_"), names(MOI.Test; all = true))
           clashes = Dict{Symbol,Vector{Symbol}}(
               t => Symbol[t2 for t2 in tests if occursin("$t", "$t2") && t != t2] 
               for t in tests
           )
           for (k, v) in clashes
               if isempty(v)
                   delete!(clashes, k)
               end
           end
       end

julia> clashes
Dict{Symbol, Vector{Symbol}} with 47 entries:
  :test_conic_PositiveSemidefiniteConeSquare_VectorAffineFunction         => [:test_conic_PositiveSemidefiniteConeSquare_VectorAffineFunction_2]
  :test_linear_complex_Zeros                                              => [:test_linear_complex_Zeros_duplicate]
  :test_model_Name                                                        => [:test_model_Name_VariableName_ConstraintName]
  :test_infeasible_MIN_SENSE                                              => [:test_infeasible_MIN_SENSE_offset]
  :test_conic_PositiveSemidefiniteConeTriangle                            => [:test_conic_PositiveSemidefiniteConeTriangle_3, :test_conic_PositiveSemidefiniteConeTriangle_VectorAffineFunction, :test_conic_PositiveSemidefiniteConeTriangle_VectorAffineFunction_2, :test_con…
  :test_multiobjective_vector_affine_function                             => [:test_multiobjective_vector_affine_function_delete, :test_multiobjective_vector_affine_function_delete_vector, :test_multiobjective_vector_affine_function_modify]
  :test_conic_linear_VectorOfVariables                                    => [:test_conic_linear_VectorOfVariables_2]
  :test_variable_delete_Nonnegatives                                      => [:test_variable_delete_Nonnegatives_row]
  :test_conic_NormSpectralCone                                            => [:test_conic_NormSpectralCone_2]
  :test_conic_RotatedSecondOrderCone_INFEASIBLE                           => [:test_conic_RotatedSecondOrderCone_INFEASIBLE_2]
  :test_solve_conflict_zeroone                                            => [:test_solve_conflict_zeroone_2]
  :test_modification_incorrect                                            => [:test_modification_incorrect_VariableIndex]
  :test_conic_SecondOrderCone_negative_post_bound                         => [:test_conic_SecondOrderCone_negative_post_bound_2, :test_conic_SecondOrderCone_negative_post_bound_3]
  :test_linear_INFEASIBLE                                                 => [:test_linear_INFEASIBLE_2]
  :test_conic_NormNuclearCone                                             => [:test_conic_NormNuclearCone_2]
  :test_nonlinear_hs071                                                   => [:test_nonlinear_hs071_NLPBlockDual, :test_nonlinear_hs071_hessian_vector_product, :test_nonlinear_hs071_no_hessian]
  :test_multiobjective_vector_affine_function_delete                      => [:test_multiobjective_vector_affine_function_delete_vector]
  :test_conic_GeometricMeanCone_VectorAffineFunction                      => [:test_conic_GeometricMeanCone_VectorAffineFunction_2, :test_conic_GeometricMeanCone_VectorAffineFunction_3]
  :test_conic_PositiveSemidefiniteConeTriangle_VectorOfVariables          => [:test_conic_PositiveSemidefiniteConeTriangle_VectorOfVariables_2]
  :test_conic_LogDetConeTriangle                                          => [:test_conic_LogDetConeTriangle_VectorAffineFunction, :test_conic_LogDetConeTriangle_VectorOfVariables]
  :test_linear_VectorAffineFunction                                       => [:test_linear_VectorAffineFunction_empty_row]
  :test_constraint_ZeroOne_bounds                                         => [:test_constraint_ZeroOne_bounds_2, :test_constraint_ZeroOne_bounds_3]
  :test_conic_RootDetConeTriangle                                         => [:test_conic_RootDetConeTriangle_VectorAffineFunction, :test_conic_RootDetConeTriangle_VectorOfVariables]
  :test_variable_add_variable                                             => [:test_variable_add_variables]
  :test_conic_GeometricMeanCone_VectorOfVariables                         => [:test_conic_GeometricMeanCone_VectorOfVariables_2, :test_conic_GeometricMeanCone_VectorOfVariables_3]
  :test_conic_linear_INFEASIBLE                                           => [:test_conic_linear_INFEASIBLE_2]
  :test_conic_PositiveSemidefiniteConeTriangle_VectorAffineFunction       => [:test_conic_PositiveSemidefiniteConeTriangle_VectorAffineFunction_2]
  :test_linear_integration                                                => [:test_linear_integration_2, :test_linear_integration_Interval, :test_linear_integration_delete_variables, :test_linear_integration_modification]
  :test_multiobjective_vector_of_variables                                => [:test_multiobjective_vector_of_variables_delete, :test_multiobjective_vector_of_variables_delete_all, :test_multiobjective_vector_of_variables_delete_vector]
  :test_infeasible_affine_MAX_SENSE                                       => [:test_infeasible_affine_MAX_SENSE_offset]
  :test_infeasible_affine_MIN_SENSE                                       => [:test_infeasible_affine_MIN_SENSE_offset]
  :test_infeasible_MAX_SENSE                                              => [:test_infeasible_MAX_SENSE_offset]
  :test_multiobjective_vector_of_variables_delete                         => [:test_multiobjective_vector_of_variables_delete_all, :test_multiobjective_vector_of_variables_delete_vector]
  :test_conic_RootDetConeSquare                                           => [:test_conic_RootDetConeSquare_VectorAffineFunction, :test_conic_RootDetConeSquare_VectorOfVariables]
  :test_conic_PositiveSemidefiniteConeSquare_VectorOfVariables            => [:test_conic_PositiveSemidefiniteConeSquare_VectorOfVariables_2]
  :test_conic_Exponential_hard                                            => [:test_conic_Exponential_hard_2]
  :test_nonlinear_objective                                               => [:test_nonlinear_objective_and_moi_objective_test]
  :test_multiobjective_vector_quadratic_function                          => [:test_multiobjective_vector_quadratic_function_delete, :test_multiobjective_vector_quadratic_function_delete_vector, :test_multiobjective_vector_quadratic_function_modify]
  :test_unbounded_MAX_SENSE                                               => [:test_unbounded_MAX_SENSE_offset]
  :test_conic_linear_VectorAffineFunction                                 => [:test_conic_linear_VectorAffineFunction_2]
  :test_unbounded_MIN_SENSE                                               => [:test_unbounded_MIN_SENSE_offset]
  :test_conic_LogDetConeSquare                                            => [:test_conic_LogDetConeSquare_VectorAffineFunction, :test_conic_LogDetConeSquare_VectorOfVariables]
  :test_linear_DUAL_INFEASIBLE                                            => [:test_linear_DUAL_INFEASIBLE_2]
  :test_variable_delete                                                   => [:test_variable_delete_Nonnegatives, :test_variable_delete_Nonnegatives_row, :test_variable_delete_SecondOrderCone, :test_variable_delete_variables]
  :test_solve_DualStatus_INFEASIBILITY_CERTIFICATE_VariableIndex_LessThan => [:test_solve_DualStatus_INFEASIBILITY_CERTIFICATE_VariableIndex_LessThan_max]
  :test_conic_NormOneCone                                                 => [:test_conic_NormOneCone_INFEASIBLE, :test_conic_NormOneCone_VectorAffineFunction, :test_conic_NormOneCone_VectorOfVariables]
  :test_multiobjective_vector_quadratic_function_delete                   => [:test_multiobjective_vector_quadratic_function_delete_vector]

We can't change the current behavior, be could add a new way to match tests exactly.

odow commented 1 year ago

See https://github.com/jump-dev/MathOptInterface.jl/pull/2129

blegat commented 1 year ago

We should probably rename these tests by adding a suffix, it wouldn't be breaking since ignoring these names will ignore it with the suffix as well.

odow commented 1 year ago

My plan was that instead of excluding "test_conic_PositiveSemidefiniteConeTriangle", you'd now exclude r"^test_conic_PositiveSemidefiniteConeTriangle$". I don't think we need to change anything in MOI.

blegat commented 1 year ago

My plan was that instead of excluding "test_conic_PositiveSemidefiniteConeTriangle", you'd now exclude r"^test_conic_PositiveSemidefiniteConeTriangle$". I don't think we need to change anything in MOI.

How do you prevent people from making the mistake ? I don't want to have to check everything I'm excluding a test that the name is not a subset or to have to use regexp. Maybe have a warning in case the user passes a string that is both an exact match and also occursin ? The warning would recommend the user to be explicit with a regexp.

odow commented 1 year ago

How do you prevent people from making the mistake ?

From now on, you just always use the regex if you want to exclude an exact test.

blegat commented 1 year ago

From now on, you just always use the regex if you want to exclude an exact test.

Is there any example of tests that are excluded with the occursin behavior outside of MOI ? All solvers are always excluding by exact match, that's very common. We can't prevent people from now knowing at that and not using regexp in which case they will loose a large coverage unexpectedly.

odow commented 1 year ago

Is there any example of tests that are excluded with the occursin behavior outside of MOI ?

https://github.com/jump-dev/Gurobi.jl/blob/258de8bec4e7f39135cd235d51749171c9f40f22/test/MOI/MOI_wrapper.jl#L61-L67

blegat commented 1 year ago

I still think that a warning would be helpful but it does not have to be blocking for https://github.com/jump-dev/MathOptInterface.jl/pull/2132