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

[Issue 168] Vectorized KKR internals #169

Merged
merged 2 commits into from
May 6, 2024
Merged

Conversation

IruNikZe
Copy link
Contributor

@IruNikZe IruNikZe commented May 6, 2024

This PR leverages NumPy's broadcasting to speed up the computation of the Kramers-Kronig-Relationship by making most of the computations happen at C-level rather than inside the for-loop that handles both odd and even case with an expensive if.
Doing so would even allow for a simple Numba-compilation to speed up the computations even further (given it is available at runtime).

It tackles this issue

Fixes #168

@IruNikZe
Copy link
Contributor Author

IruNikZe commented May 6, 2024

A minor refactoring is using x * x rather than x ** 2 because it does not involve the power functions which makes it faster.

@IruNikZe IruNikZe changed the title Vectorized KKR internals [Issue 168] Vectorized KKR internals May 6, 2024
Copy link
Member

@domna domna left a comment

Choose a reason for hiding this comment

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

Thank you @IruNikZe for this nice improvement of the kkr :) Indeed, this was never really optimised for speed.

Fixes #168

@domna domna merged commit 07fa0b3 into PyEllips:master May 6, 2024
10 checks passed
@IruNikZe
Copy link
Contributor Author

IruNikZe commented May 6, 2024

Of course! 👍 Thanks for the quick response

@domna
Copy link
Member

domna commented May 6, 2024

Of course! 👍 Thanks for the quick response

Sure. I could also do a small release if you need this directly from the package or are you using this through a git install anyways?

@IruNikZe
Copy link
Contributor Author

IruNikZe commented May 7, 2024

I can use it via Git, that's fine by now 👨‍💻
Thanks for the offer 👍

@laarisyko
Copy link

It would be super awesome if this feature could be released :D Thank you for your work.

@MarJMue
Copy link
Collaborator

MarJMue commented Jun 20, 2024

Hey @laarisyko,
Please excuse, that we didn't release it right away.
I am preparing everything right now.

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.

KKR internals not vectorized
4 participants