dgasmith / opt_einsum

⚡️Optimizing einsum functions in NumPy, Tensorflow, Dask, and more with contraction order optimization.
https://dgasmith.github.io/opt_einsum/
MIT License
863 stars 68 forks source link

updates to 'dp' optimizer #119

Closed jcmgray closed 4 years ago

jcmgray commented 5 years ago

Description

These updates the 'dp' optimizer in a few ways:

I think with these, it would make sense to make the 'optimal' algorithm point at 'dp' since with the outer product search it should now find all the same contractions, and with the cost_cap turned off it's as fast for small contractions as well. This would fix #99. Thoughts @dgasmith? Also cc @mrader1248.

Todos

Status

codecov[bot] commented 5 years ago

Codecov Report

Merging #119 into master will increase coverage by 0.05%. The diff coverage is 100%.

mrader1248 commented 5 years ago

Can you give me an example, where it is beneficial to construct outer products?

mrader1248 commented 5 years ago

Sorry, it took me a while to have a look at this. Overall, LGTM. And especially thanks for fixing this stupid smallest dimension 1 bug.

jcmgray commented 5 years ago

Can you give me an example, where it is beneficial to construct outer products?

I've put this example in the tests (taken from you in another thread I think!):

def test_custom_dp_can_optimize_for_outer_products():
    eq = "a,b,abc->c"

    da, db, dc = 2, 2, 3
    shapes = [(da,), (db,), (da, db, dc)]

    opt1 = oe.DynamicProgramming(search_outer=False)
    opt2 = oe.DynamicProgramming(search_outer=True)

    info1 = oe.contract_path(eq, *shapes, shapes=True, optimize=opt1)[1]
    info2 = oe.contract_path(eq, *shapes, shapes=True, optimize=opt2)[1]

    assert info2.opt_cost < info1.opt_cost

Of course in practice I imagine it's almost never useful. However I think it would make sense for 'dp' to replace the optimal implementation, and if that happens then the option to be guaranteed optimal seems beneficial for testing edge cases etc. Plus for v small sizes there's no penalty hit.

@dgasmith do you have any thoughts on pointing 'optimal' to 'dp'? Maybe in a later PR.

jcmgray commented 5 years ago

@dgasmith Also happy to have a go a adding more docs (e.g. each variable etc) if you think necessary.

mrader1248 commented 4 years ago

@jcmgray I see, I thought you found an example where constructing outer products first leads to a better solution even in the asymptotic limit. Maybe it would be good to mention in the docs that allowing for intermediate outer products can lead to horrible optimisation times?

jcmgray commented 4 years ago

@jcmgray I see, I thought you found an example where constructing outer products first leads to a better solution even in the asymptotic limit

Well in the asymptotic limit where da and db are fixed but dc get's larger, indeed the outer product is still necessary here.

Maybe it would be good to mention in the docs that allowing for intermediate outer products can lead to horrible optimisation times?

I've mentioned this in the docstring for the search_outer option, but yes might be worth adding another warning to the main docs as well.

jcmgray commented 4 years ago

OK, so I've added a warning to the 'dp' docs about turning on the outer product search, and factored out the single term parsing of inputs. Unless there are more comments I'll merge in a bit.