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

Cleanup and performance improvements #20

Merged
merged 8 commits into from
Dec 19, 2017
Merged

Conversation

rdeits
Copy link
Contributor

@rdeits rdeits commented Dec 19, 2017

Better version of #19

This does the following:

  1. Replaces the non-const globals user_f and user_j with const Refs, using FunctionWrappers to ensure type-stability
  2. Removes all the custom sparse_matrix code, because Julia's built-in SparseMatrixCSC does the same thing
  3. Removes additional solveMCP methods which exist only to provide default arguments, because default argument values already create those extra methods automatically.

I did some profiling with the following benchmark:

using PATHSolver
using BenchmarkTools

function construct_problem()
    srand(1)
    n = 50
    M = sprandn(n, n, 0.1)
    M += M'
    q = randn(n)
    myfunc(x) = M*x + q

    lb = zeros(n)
    ub = fill(100.0, n)
    myfunc, lb, ub
end

myfunc, lb, ub = construct_problem()
results = @benchmark solveMCP($myfunc, $lb, $ub) setup=(options(convergence_tolerance=1e-2, output=:no)) evals=1
display(results)

Before this PR, I saw about 2.67 ms, and after this PR I see 2.18 ms, or about a 20% improvement in total time. That time includes all the time spent in PATH and computing jacobians, so the actual improvement in callback overhead should be better than 20%.

@coveralls
Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-4.3%) to 95.652% when pulling 51a9e2b on rdeits:cleanup into 7e4928a on chkwon:master.

@rdeits
Copy link
Contributor Author

rdeits commented Dec 19, 2017

Ok, I've also fixed compatibility with Julia v0.5, v0.6, and nightly on Linux. Appveyor is broken with Julia nightly due to JuliaLang/julia#20899

@coveralls
Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-3.8%) to 96.25% when pulling 5c49e40 on rdeits:cleanup into 7e4928a on chkwon:master.

@chkwon
Copy link
Owner

chkwon commented Dec 19, 2017

This looks cool! Thanks for PR.

@coveralls
Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-4.2%) to 95.833% when pulling eb13c31 on rdeits:cleanup into 7e4928a on chkwon:master.

@chkwon chkwon merged commit a44fe04 into chkwon:master Dec 19, 2017
@coveralls
Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-4.3%) to 95.652% when pulling 700436c on rdeits:cleanup into 7e4928a on chkwon:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants