Skip to content

remove final permutedims() in binary_einsum#58

Open
starsfordummies wants to merge 5 commits intomasterfrom
speed4free
Open

remove final permutedims() in binary_einsum#58
starsfordummies wants to merge 5 commits intomasterfrom
speed4free

Conversation

@starsfordummies
Copy link
Contributor

maybe I don't fully understand the logic in all those index sets in binary_einsum, but if we can get rid of the final permutedims() that would give a significant speedup. The tests hopefully will help see if I'm wrong

@jofrevalles
Copy link
Member

Oh, so we could put something that reorders the indices instead of the inner array, right?

Copy link
Member

@mofeing mofeing left a comment

Choose a reason for hiding this comment

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

I'm thinking that we can probably better avoid kwargs and directly use the ScopedValue as a performance configuration. Tell me if it's difficult for you and I can do it.

Comment on lines +54 to +55


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

@mofeing
Copy link
Member

mofeing commented Aug 5, 2025

Oh, so we could put something that reorders the indices instead of the inner array, right?

yeah, kind of. we wouldn't really force an order doing permutedims on the array, but the other way around. ofc, this would be optional and would be enabled it by default.

@starsfordummies
Copy link
Contributor Author

I still don't know if it makes sense to add a global option to force contract to return indices in a given order, when in most cases it decides the order by itself - know what I mean?
The way I implemented it right now it should force the order just when the out=.. list is given (which I understand is when you want to get a specific order of the outgoing legs, right? otherwise I don't understand what it's for)

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