3 ms·

Exactly, that is the real issue, vectorization might be good enough in terms of performance. It doesn't seem to be mentioned in the article at all.

by rwalle 4y ago

hn92726819 4y ago
It might have been added later, but the author mentions vectorization in the beginning: > It’s worth noting that converting parts of / everything to vectorized numpy might be possible for this toy library, but will be nearly impossible for the real library while making the code much less readable and modifiable, and the gains are going to be limited (here’s a partially vertorized version, which is faster but far from the results we are going to achieve).
- akasakahakada 4y ago
  Semi Vectorized code: https://github.com/ohadravid/poly-match/blob/main/poly_match_v1_5_vectorized.py https://github.com/ohadravid/poly-match/blob/main/poly_match... Expecting Python engineers unable to read defacto standard numpy code but meanwhile expect everyone can read Rust..... Not to mention that the semi-vectorized code is still suboptimal. Too many for loops despite the author clearly know they can all be vectorized. For example instead the author can just write something like: np.argmin( distances[distances<=threshold] ) Also in oneplace there is: np.xxx( np.xxx, np.xxx + np.xxx) You can just slap numexpr on top of it to compile this line on the fly. https://github.com/pydata/numexpr https://github.com/pydata/numexpr
  - Tarq0n 4y ago
    Not super familiar with Python but isn't that append call within a loop going to cause a lot of allocations?
    - akasakahakada 4y ago
      3000 calls to list.append() cost only 2ms. In a computational intense program, no one bothers. Because usually one call to do matrix multiplication is already costing 500ms or so. Of couse you can prelocate memory for size=3000, and append stuff in a loop. But this saves only 10ms. Too insignificant.
  - ohr 4y ago
    Author here: For the original library we did all the numpy tricks we could think of, but we really needed to do this type of exhaustive search for some of the data. If someone wants to open a PR with a "fully optimized" numpy code, that would be very cool just for comparison :)

Exactly, that is the real issue, vectorization might be good enough in terms of performance. It doesn't seem to be mentioned in the article at all.

by rwalle 4y ago

Exactly, that is the real issue, vectorization might be good enough in terms of performance. It doesn't seem to be mentioned in the article at all.

hn92726819 4y ago

It might have been added later, but the author mentions vectorization in the beginning: > It’s worth noting that converting parts of / everything to vectorized numpy might be possible for this toy library, but will be nearly impossible for the real library while making the code much less readable and modifiable, and the gains are going to be limited (here’s a partially vertorized version, which is faster but far from the results we are going to achieve).

akasakahakada 4y ago
Semi Vectorized code: https://github.com/ohadravid/poly-match/blob/main/poly_match_v1_5_vectorized.py https://github.com/ohadravid/poly-match/blob/main/poly_match... Expecting Python engineers unable to read defacto standard numpy code but meanwhile expect everyone can read Rust..... Not to mention that the semi-vectorized code is still suboptimal. Too many for loops despite the author clearly know they can all be vectorized. For example instead the author can just write something like: np.argmin( distances[distances<=threshold] ) Also in oneplace there is: np.xxx( np.xxx, np.xxx + np.xxx) You can just slap numexpr on top of it to compile this line on the fly. https://github.com/pydata/numexpr https://github.com/pydata/numexpr
- Tarq0n 4y ago
  Not super familiar with Python but isn't that append call within a loop going to cause a lot of allocations?
  - akasakahakada 4y ago
    3000 calls to list.append() cost only 2ms. In a computational intense program, no one bothers. Because usually one call to do matrix multiplication is already costing 500ms or so. Of couse you can prelocate memory for size=3000, and append stuff in a loop. But this saves only 10ms. Too insignificant.
- ohr 4y ago
  Author here: For the original library we did all the numpy tricks we could think of, but we really needed to do this type of exhaustive search for some of the data. If someone wants to open a PR with a "fully optimized" numpy code, that would be very cool just for comparison :)