Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce allocations and improve performance in syevr! #1176

Open
wants to merge 3 commits into
base: release-1.11
Choose a base branch
from

Conversation

BioTurboNick
Copy link

@BioTurboNick BioTurboNick commented Jan 20, 2025

Profiling code that made heavy use of eigen, creating the final arrays here was a hot spot.

I imagine the @view is going to get a frown... it's strictly reducing the second dimension so in theory it could be resized in-place, but I don't know of a way to do that? EDIT: No longer using a view

const AAA = [0.9124359407098924 0.9654684977248279 0.9237069034235668 0.4479855053248103; 0.6313189619311209 0.039044817699327994 0.21147802367801194 0.2242156559761067; 0.9346485775560693 0.7639560598623737 0.30303677867123346 0.33048614822558353; 0.5034773545892783 0.21614932409092613 0.37837816060665885 0.6122920572414832]

function f(a)
      for i = 1:1000
           eigen(Symmetric(a))
      end
end

Before:
image

After:
image

@andreasnoack
Copy link
Member

It would he good to include the before and after profiling results here to give an idea about the impact

@BioTurboNick
Copy link
Author

BioTurboNick commented Jan 22, 2025

Thanks, added. Figured I should also do Hamiltonian too because it's the exact same situation.

Looks like there are other spots that could benefit from this as well.

EDIT: found an alternative to @view: initialize Z as a vector and then reshape(resize!()) at the end; extra allocations, but still fewer than the original, and similar performance to the view version.

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.

2 participants