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

add real vectorized version and numba version #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wkirgsn
Copy link

@wkirgsn wkirgsn commented Apr 24, 2023

I felt the world of vectorized python was not appropriately represented.
I added a v1.6 that comes with a pure numpy and also numba version.
The numba version is slower, so there is still room for improvement, probably.
However, the numpy version is "only" two times slower than the full rust implementation.

I would be glad to discuss the solution and whether it could be merged in one way or the other into the poly-match example.

Having said that, I love Rust and I would like to see more examples where Rust outshines Python in numerical simulation/ computation.

@ohadravid
Copy link
Owner

ohadravid commented Apr 25, 2023

Hi @wkirgsn 🙂

I'd love to have the examples in the repo, awesome work 🚀

A few suggestions:

  1. To make the comparison "fair", the polygon reshaping should be part of the measured main (IMO) [for the original library, the list change each time so we need to do the prepossessing each time]
  2. I like your comments, and if you don't mind also explaining in a few place how does it do it / linking to the relevant numpy docs
  3. numba: I think using prange isn't equivalent to the original (at least in this case), and I would prefer a separate version (in a separate file) that speeds up one of the "regular" Python versions (the most vectorized numpy version doesn't benefit from it because it's already mostly C).

Other than that, you can also modify the readme to include your relative timings and add a bit of an explanation about the code (I don't think we need the notebook). You can add you name and note that this was added later after the post was published.

Thanks! Hope you enjoyed the article 😁

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