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

Support higher-dimensional arrays in dot #85

Merged
merged 1 commit into from
Oct 9, 2018
Merged

Support higher-dimensional arrays in dot #85

merged 1 commit into from
Oct 9, 2018

Conversation

timholy
Copy link
Contributor

@timholy timholy commented Oct 7, 2018

I'm implementing the earth mover distance and the most natural representation uses a matrix of Variables and requires taking its dot-product with some weights.

Incidentally, in profiling I'm finding there's still quite a lot of allocation, which I'm guessing is a consequence of this line. Any thoughts on how to get around it? Also this line is insanely slow. Any tips for overcoming such problems?

@codecov
Copy link

codecov bot commented Oct 7, 2018

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #85   +/-   ##
=======================================
  Coverage   97.86%   97.86%           
=======================================
  Files           9        9           
  Lines         515      515           
=======================================
  Hits          504      504           
  Misses         11       11
Impacted Files Coverage Δ
src/functions.jl 99.24% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update daca170...e8e591b. Read the comment docs.

show(buf, x ⋅ w)
@test String(take!(buf)) == "0.1 * x1 + 0.3 * x2 + 0.2 * x3 + 0.4 * x4 + 0.0"
show(buf, w ⋅ x)
@test String(take!(buf)) == "0.1 * x1 + 0.3 * x2 + 0.2 * x3 + 0.4 * x4 + 0.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I like involving show in this test. So far I've usually just evaluated the expressions with random variable values and ensured that the results are the same. But I think we should just overload == (and hash) so that 0.1 * x1 + 0.2 * x2 + 0.0 == 0.2 * x2 + 0.1 * x1 + 0.0 == 0.1 * x1 + 0.1 * x2 + 0.1 * x2 + 0.0. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once #86 is merged, could you use that here instead?

@tkoolen
Copy link
Owner

tkoolen commented Oct 8, 2018

The MathOptInterface wrappers for Gurobi (and other LQOI-based solvers) currently are just quite inefficient. Again, if your problem doesn't involve integer variables, try OSQP. If not (or maybe, regardless), you'll have to open issues with the MOI wrapper packages. The zero allocation results in the readme are of course predicated on having a solver interface that doesn't allocate when you do problem modification.

@timholy
Copy link
Contributor Author

timholy commented Oct 9, 2018

Done.

I keep getting fooled by the Q in OSQP, forgetting that of course it can solve linear problems too. It's definitely better than Gurobi or GLPK. Though I've now benchmarked it against https://github.com/robertfeldt/EarthMoversDistance.jl.git and getting results like these:

# bins time OSQP (μs) time custom C library (μs)
6 140 3
51 7860 160

So I'll probably be using the dedicated solver.

@tkoolen tkoolen merged commit 861f5cf into tkoolen:master Oct 9, 2018
@tkoolen
Copy link
Owner

tkoolen commented Oct 9, 2018

Great, thanks. Yeah, OSQP is definitely best when the cost function Hessian is positive definite, and a proper dedicated LP algorithm is always going to win.

@timholy timholy deleted the teh/mdot branch October 9, 2018 11:58
@timholy
Copy link
Contributor Author

timholy commented Oct 9, 2018

Interestingly, at least for the small number of bins I think OSQP beat the linear solvers. But everyone got beat by the specialized implementation that takes advantage of particular features of "transportation problems."

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.

2 participants