ahwillia / Einsum.jl

Einstein summation notation in Julia
MIT License
151 stars 17 forks source link

Multi-threaded macro #29

Closed mcabbott closed 5 years ago

mcabbott commented 5 years ago

This adds a new macro which, instead of adding @simd to the innermost loop, adds Threads.@threads to the outermost. For this to be faster on some test cases, it writes directly into the array, rather than defining local accumulators, as now shown in the readme.

I added this macro to a few of the tests. Some other tests fail on 1.0, namely things like @einsum A[i] := X[i+:offset]. I tried for a bit to make the function unquote_offsets work correctly, but failed, so for now I have simply commented out those tests. I also edited .travis to test on 1.0, and added Compat so that the remaining tests now pass on both 0.6 and 1.0.

ahwillia commented 5 years ago

Looks good - I'll merge this so that others can see it. Note that I'm not interested in maintaining this package in an official capacity at this time -- that is, I won't register it for julia 1.0+ but you should feel free to fork it and do so if you want.

mcabbott commented 5 years ago

Thanks!

I understand about maintenance and time. Does anyone else have permission to commit things, and tag new releases?

phipsgabler commented 5 years ago

@mcabbott I have push access as well. A couple of months ago I had time and motivation to rewrite a few things in my fork, but then became lazy again (and forgot most of the stuff I wrote there...)

My original plan was to change the workings to use some common abstract representation for an index expression and convert that to loops, similar to what TensorOperations does. But if someone wants to brush up and update things, I'm willing to participate.

After all, it would be nice to still have a working package which allows to use non-strided arrays and non-contracting operations. On the other hand, the TensorOperations maintainer has repeatedly claimed that they are open for adding support for such operations.

mcabbott commented 5 years ago

OK, good to know -- this package seems valuable to keep alive, as simple loops are the right way to do some things, despite being a terrible way to do matrix multiplication.

I got interested on realising that some calculations I was doing by broadcasting (into some huge tensor) and then reducing, could be made both faster and more readable using @einsum. Mine tend to have some exp or log functions, expensive enough that @threads helps. I don't see how to squeeze such operations into how TensorOperations acts right now, although maybe there's a way.

For now though, let's see whether adding this PR has created any problems, before tagging. I don't think it should have affected the output of ordinary @einsum at all.