jump-dev / JuMP.jl

Modeling language for Mathematical Optimization (linear, mixed-integer, conic, semidefinite, nonlinear)
http://jump.dev/JuMP.jl/
Other
2.24k stars 396 forks source link

Add better tests for shapes #3808

Closed odow closed 2 months ago

odow commented 2 months ago

We could test that the dot product before and after reshaping is the same.

This would catch issues like https://github.com/jump-dev/JuMP.jl/pull/3797

odow commented 2 months ago

@blegat: could you sketch what you had in mind here? Adding some default tests for the shapes doesn't help us show that we use the appropriate shape for a constraint? (Which was the issue here.)

blegat commented 2 months ago

I think it can just be something like

function test_shape(primal, dual, shape)
    vec_primal = vectorize(primal, shape)
    vec_dual = vectorize(dual, dual_shape(shape))
    @test primal = reshape(vec_primal, shape)
    @test dual == reshape(vec_dual, dual_shape(shape))
    @test dot(primal, dual) == dot(vec_primal, vec_dual)
end
odow commented 2 months ago

Sure. But this doesn't really help us catch the fact that the solver might return a "different" value to the one we expected (if we didn't think clearly) for the dual. That would just tell us that Symmetric works. But it would't help tell us we needed the adjoint? (Unless we know good values for primal and dual to test?)

blegat commented 2 months ago

You're right, then we can do something like: I think it can just be something like

function test_shape(primal, dual, con::VectorConstraint)
    vec_primal = vectorize(primal, con.shape)
    vec_dual = vectorize(dual, dual_shape(con.shape))
    @test primal = reshape(vec_primal, con.shape)
    @test dual == reshape(vec_dual, dual_shape(con.shape))
    @test dot(primal, dual) == MOI.Utilities.set_dot(vec_primal, vec_dual, con.set)
end