-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
ENH: Improve performance of tril_indices and triu_indices #18176
Conversation
nonzero is slow. Use np.indices and np.broadcast_to to speed it up.
you should compare new implements in |
I don't think #18183 is related, can you add your crash logs here? From the CI perspective, The only failure is |
Oh sorry I didn't mean that the pull request was related to the CI issue. But that the solution in #18183 might make nonzero on par with my solution. I don't get how TestMatmul.test_vector_matrix_values can crash when I add some performance tests. I'll restart and see if it's just some CI hickup. |
Can you provide the benchmark result by running |
@Qiyu8 I can't figure out how to do it. Do you want to run it yourself or act IT-support? Anaconda prompt:
Ok, go back one level then:
|
you can read the benchmark guide here. |
@Qiyu8 It's still not working for me, got a little further though:
Also virtualenv doesn't seem to be the recommended way anymore:
|
@Qiyu8 I never got Illviljan-faster_tril_indices:
master:
|
The performance improvements looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one small styling suggestion.
Thanks @Illviljan |
np.nonzero
seems slow compared to other methods. Usenp.indices
andnp.broadcast_to
to speed it up.Closes #18153.
Performance comparison: