search

ahwillia / Einsum.jl

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

Added arbitrary RHS expressions; @fastcopy; inbounds; removed create … #3

Closed StephenVavasis closed 8 years ago

StephenVavasis commented 8 years ago

…option

ahwillia commented 8 years ago

Looks like a great start! I'll wait on the tests before reviewing. It might help to break this up into multiple pull requests since you seem to be adding a lot of new functionality.

Question: is @fastcopy faster than copy! already provided in base?

I like the inbounds stuff -- this was definitely on my to do list.

StephenVavasis commented 8 years ago

Alex,

As far as I know, Base.copy! cannot deal with subarrays directly. To use it on a subarray, you have to call it in conjunction with sub, and therefore pay the price of a heap allocation.

StephenVavasis commented 8 years ago

Alex,

With regard to the create operation, it seems to make sense only in the case that the LHS is of the form x[a,b,c,d,..] where a,b,c,d are distinct variable names (no expressions; no repeats allowed). Do you agree? In this case, I think it would be straightforward to put this functionality back.

WIth regard to splitting the PR into separate PR's, this is too complicated for me. I have spent way too many hours trying to get my head around git. But if you want to take a crack at it, feel free!

ahwillia commented 8 years ago

Yes I think these constraints on the LHS make sense. The only reasonable alternative would be to assume that all entries are zero by default e.g. x[i,i] = y[i] creates a diagonal matrix.

Don't worry about multiple PRs, just get it working for what you need and I can sort through it once the tests are in place. On May 23, 2016 9:11 PM, "StephenVavasis" notifications@github.com wrote:

Alex,

With regard to the create operation, it seems to make sense only in the case that the LHS is of the form x[a,b,c,d,..] where a,b,c,d are distinct variable names (no expressions; no repeats allowed). Do you agree? In this case, I think it would be straightforward to put this functionality back.

WIth regard to splitting the PR into separate PR's, this is too complicated for me. I have spent way too many hours trying to get my head around git. But if you want to take a crack at it, feel free!

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/ahwillia/Einsum.jl/pull/3#issuecomment-221161867

ahwillia commented 8 years ago

This was superseded by a series of recent pull requests(https://github.com/ahwillia/Einsum.jl/pull/7, https://github.com/ahwillia/Einsum.jl/pull/9, https://github.com/ahwillia/Einsum.jl/pull/12). Nearly all this functionality is soon to be supported.