Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing the Convex.jl Problem Depot #328

Merged
merged 12 commits into from
Oct 28, 2019

Conversation

ericphanson
Copy link
Collaborator

@ericphanson ericphanson commented Sep 9, 2019

What is it?

A repository of Convex.jl problems that can be used internally or externally to Convex.jl for testing and benchmarking.

Why?

When I added benchmarks in #321, I copied over some of the tests, and edited them by hand to make them usable as benchmarks. Since that was somewhat labour-intensive, I only introduced some SDP and affine problems as tests. It seemed like that wasn't the ideal solution, but I wanted to get some benchmarks up.

Simultaneously, over at SDPA_GMP.jl, @JiazhengZhu and I were thinking about how to test the solver we were wrapping. We thought the MOI tests were nice, but we also wanted end-to-end testing from formulation to solution, like Conevx.jl has in its tests (since I noticed this can find problems that can be missed otherwise, e.g. jump-dev/SDPA.jl#17). @JiazhengZhu did the labour of copying over the problems and modifying them to allow arbitrary numeric types, etc, for use with SDPA-GMP.

Then, a third testing-related issue arose. Over at https://github.com/ericphanson/Convex.jl on the MathOptInterface branch, I was looking at testing Convex.jl with MOI and MPB simultaneously. The simplest way to do that was to use the solvers vector to hold both MOI and MPB solvers. Soon I found, however, that when a test failed, it was hard to figure out if the test was using an MOI or an MBP solver. I thought it would be better to run all the tests with MPB, then all the tests again with MOI. But including the files twice with different choices of the global variable solver did not work, since include evaluates in global scope, and it seemed like a bad idea anyway.

The ProblemDepot in this PR is a unified way to fulfill all those needs. This creates a repository of problems in the Convex.jl source, in a submodule called ProblemDepot, along with helper functions to easily use those problems for testing and benchmarking. This way, each problem is only written down once in a flexible way so that it can be used for all the above use cases.

How to use it?

I think it quite easy to use (modelled after the MOI tests), and I've added documentation in this PR as well. But to get a quick look right away, here is the main content of the runtests.jl file:

@testset "Convex" begin
    include("test_utilities.jl")

    @testset "SCS" begin
        run_tests(; exclude=[r"mip"]) do p
            solve!(p, SCSSolver(verbose=0, eps=1e-6))
        end
    end

    @testset "ECOS" begin
        run_tests(; exclude=[r"mip", r"sdp"]) do p
            solve!(p, ECOSSolver(verbose=0))
        end
    end

    @testset "GLPK MIP" begin
        run_tests(; exclude=[r"socp", r"sdp", r"exp"]) do p
            solve!(p, GLPKSolverMIP())
        end
    end
end

First, one can see that it's easy to run all the tests (up to some exclusions specified by regex) for each solver separately, in their own testset. Each "problem" being tested lives in its own function, and run_tests calls all of those functions in a loop, placing each in a testset (copying the structure from the current tests).

Second, regex-excludes allow nuanced problem removal. The old system was to gate a large chunk of tests behind a check of e.g. can_solve_sdp(solver). This has two problems: one, one cannot tell statically which tests will be called by which solver; instead one needs to know the result of can_solve_sdp(solver) which has a call to MPB (edit: this is really just a problem of explicitness). And two, it does not allow for much nuance; for example, some tests involve both SDP and SOCP constraints (via norm objectives). With the regex excludes, one just has to put each cone into the group or name of the problem, and it will be excluded.

Another example is the main line from benchmark/benchmarks.jl:

const SUITE = BenchmarkGroup()
SUITE["formulation"] = Convex.ProblemDepot.benchmark_suite(Convex.conic_problem)

Here, we define a BenchmarkGroup, and generate a suite of benchmarks by passing a function to be evalauted for each. To benchmark a solver, one could use instead e.g.

SUITE["SCS"] = Convex.ProblemDepot.benchmark_suite(p -> Convex.solve!(p, SCSSolver(verbose=0)))

What has changed?

The ProblemDepot module is introduced, along with docs and some tests. The tests and benchmarks are updated to use the ProblemDepot problems. Every test has been copied over, except for Convex.clearmemory(), which I'll add the deprecation test for once #322 gets merged.

I modified the tests using the script https://gist.github.com/ericphanson/66a729fd8d8fd66b836bb6172dff0e27 + some small changes by hand afterwards.

Edit:

Just to note--
This PR adds a dependency on BenchmarkTools to faciliate using the ProblemDepot for benchmarking. However, soon Convex should depend on MOI (#330), which itself depends on BenchmarkTools. So after we switch to MOI, this just makes a transitive dependency into a dependency, which doesn't seem too bad (you have to load the code either way!).

@ericphanson
Copy link
Collaborator Author

Hmm, benchmarking timed out (50 mins), and didn't get past the tuning. I guess there are just too many problems to try to benchmark them all. We could also probably commit a tuning file instead of retuning with every PR, which should save ~half the time.

Maybe it's a bit ambitious to try to benchmark every problem. Not quite sure what to do though. If we can get the timing under 50 mins, we could add allow-failures on the benchmarks, so it doesn't hold up the green light.

@ericphanson ericphanson mentioned this pull request Sep 9, 2019
12 tasks
@ericphanson
Copy link
Collaborator Author

I think Travis benchmarks are not super useful due to noise anyway, and the best we can hope for is an easy indication that something needs looking into more. So I think it's better to do something like choose 10 problems that cover a wide range of atoms, instead of trying to just run every single problem every time. That way it should finish in a reasonable amount of time and can give a quick indication. For big PRs that touch internals a lot, the full suite should probably be run locally anyway (less noisy).

@ericphanson
Copy link
Collaborator Author

I'd like to get #330 into a reviewable state soon, since it's getting close to finished (I've got dual variables working locally now; I'll push that soon). Hence, I'd like to merge this relatively soon. I made some updates to the ProblemDepot on #330 so I'll backport these here first.

@ericphanson
Copy link
Collaborator Author

I'll merge this in an hour or so, to make #330 more reviewable.

@ericphanson ericphanson merged commit cba179f into jump-dev:master Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant