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

KKR internals not vectorized #168

Closed
IruNikZe opened this issue May 6, 2024 · 1 comment · Fixed by #169
Closed

KKR internals not vectorized #168

IruNikZe opened this issue May 6, 2024 · 1 comment · Fixed by #169

Comments

@IruNikZe
Copy link
Contributor

IruNikZe commented May 6, 2024

Hello,

When I looked at the implementation of _calc_kkr I saw that there are 2 issues when it comes to speed:

  • the code is formulated in a mathematical sense which handles the two cases of odd and even in one loop:
    integral = np.zeros(len(t))
    interval = np.diff(x, prepend=x[1] - x[0])
    odd_y, odd_x = t[1::2], x[1::2]
    even_y, even_x = t[::2], x[::2]
    for i, x_i in enumerate(x):
        if i % 2 == 0:
            integral[i] = trafo(odd_y, odd_x, x_i)
        else:
            integral[i] = trafo(even_y, even_x, x_i)

This is common for mathematically inspried code, yet the if in there is a lot of evaluations for a condition with a very predictable pattern (True, False, True, False, ....).
Making one loop for the odd and one for the even case, respectively, usually gives some speedup.
But actually, this operation can be fully vectorized leveraging NumPy's broadcasting rules.

  • integral is initalised with zeros which are never used, so initializing with empty would be slightly faster.

I have already forked the repository and rewrote the code to open a PR.

Kind regards

@IruNikZe
Copy link
Contributor Author

IruNikZe commented May 6, 2024

#169

@domna domna closed this as completed in #169 May 6, 2024
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 a pull request may close this issue.

1 participant