Skip to content

Small simplification of indexmanipulations#383

Merged
Jutho merged 2 commits intomainfrom
ld-simplify
Mar 19, 2026
Merged

Small simplification of indexmanipulations#383
Jutho merged 2 commits intomainfrom
ld-simplify

Conversation

@lkdvos
Copy link
Member

@lkdvos lkdvos commented Mar 18, 2026

This PR removes a (possibly micro-)optimization that bypassed the Strided.jl machinery for a small gain in performance. Here I removed that part again since it severely complicates things for the GPU implementations.
It's also safer in the sense that the current implementation was only correct since it was implicitly never called on conj'd StridedViews, so I do think that in the long term this is more maintainable anyways.

I will try and see if I can run the benchmark suite to actually gauge if there are any measurable performance implications here, but I would guess that this is a reasonable trade-off.

@lkdvos
Copy link
Member Author

lkdvos commented Mar 18, 2026

See also QuantumKitHub/Strided.jl#52

@kshyatt kshyatt enabled auto-merge (squash) March 19, 2026 09:41
@Jutho
Copy link
Member

Jutho commented Mar 19, 2026

I had never even noticed when this was introduced (or had forgotten about it). Was this the reason for the special copyto! implementation QuantumKitHub/Strided.jl#52 ?

@kshyatt
Copy link
Member

kshyatt commented Mar 19, 2026

@Jutho It's the reason for one of them, yes!

@lkdvos
Copy link
Member Author

lkdvos commented Mar 19, 2026

Feel free to merge without the benchmarkresults, I'm pretty sure it's not realistic for us to maintain things like this anyways.

@Jutho
Copy link
Member

Jutho commented Mar 19, 2026

@kshyatt , what is the other one?

@Jutho Jutho disabled auto-merge March 19, 2026 10:59
@Jutho Jutho merged commit 66187bb into main Mar 19, 2026
34 of 50 checks passed
@Jutho Jutho deleted the ld-simplify branch March 19, 2026 11:00
@kshyatt
Copy link
Member

kshyatt commented Mar 19, 2026

@Jutho It's in the logic for flip! The offending line is here

@kshyatt
Copy link
Member

kshyatt commented Mar 19, 2026

With the copyto! + Broadcasted changes in the Strided.jl PR, we can avoid scalar indexing for all the flip operations 🤠

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