NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

Micro-optimize rotation transform.#22108

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

timhoffm merged 1 commit intomatplotlib:mainfromanntzer:fr

Jan 5, 2022

Merged

Micro-optimize rotation transform.#22108

timhoffm merged 1 commit intomatplotlib:mainfromanntzer:fr

Jan 5, 2022

Conversation

Copy link

Contributor

anntzer commentedJan 5, 2022

The following test script shows a ~3x speedup.

importmath,numpyasnpmtx=np.array([[.1,.2,.3], [.4,.5,.6], [0,0,1]])theta=np.pi/4defrotate(mtx,theta):a=math.cos(theta)b=math.sin(theta)rotate_mtx=np.array([[a,-b,0.0], [b,a,0.0], [0.0,0.0,1.0]],float)returnnp.dot(rotate_mtx,mtx)defrfast(mtx,theta):a=math.cos(theta)b=math.sin(theta)    (xx,xy,x0), (yx,yy,y0),_=mtx.tolist()# mtx = [[a -b 0], [b a 0], [0 0 1]] * mtxmtx[0,0]=a*xx-b*yxmtx[0,1]=a*xy-b*yymtx[0,2]=a*x0-b*y0mtx[1,0]=b*xx+a*yxmtx[1,1]=b*xy+a*yymtx[1,2]=b*x0+a*y0returnmtx%timeitrotate(mtx,theta)%timeitrfast(mtx,theta)

PR Summary

PR Checklist

Tests and Styling

Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

anntzer added the Performance label

Jan 5, 2022

Micro-optimize rotation transform.

ff120cd

The following test script shows a ~3x speedup.```pythonimport math, numpy as npmtx = np.array([[.1, .2, .3], [.4, .5, .6], [0, 0, 1]])theta = np.pi / 4def rotate(mtx, theta):    a = math.cos(theta)    b = math.sin(theta)    rotate_mtx = np.array([[a, -b, 0.0], [b, a, 0.0], [0.0, 0.0, 1.0]],                          float)    return np.dot(rotate_mtx, mtx)def rfast(mtx, theta):    a = math.cos(theta)    b = math.sin(theta)    (xx, xy, x0), (yx, yy, y0), _ = mtx.tolist()    # mtx = [[a -b 0], [b a 0], [0 0 1]] * mtx    mtx[0, 0] = a * xx - b * yx    mtx[0, 1] = a * xy - b * yy    mtx[0, 2] = a * x0 - b * y0    mtx[1, 0] = b * xx + a * yx    mtx[1, 1] = b * xy + a * yy    mtx[1, 2] = b * x0 + a * y0    return mtx%timeit rotate(mtx, theta)%timeit rfast(mtx, theta)```

anntzer force-pushed thefr branch from1fd1478 toff120cdCompare

January 5, 2022 11:18

jklymak reviewed

Jan 5, 2022

View reviewed changes

Copy link

Member

jklymak left a comment•
edited
Loading

Choose a reason for hiding this comment

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

mtx isn't a pointer, is it?

Copy link

ContributorAuthor

anntzer commentedJan 5, 2022•
edited
Loading

It is. (Or rather, modifications are done in place.)

tacaswell added this to thev3.6.0 milestone

Jan 5, 2022

tacaswell approved these changes

Jan 5, 2022

View reviewed changes

timhoffm approved these changes

Jan 5, 2022

View reviewed changes

Copy link

Member

timhoffm left a comment•
edited
Loading

Choose a reason for hiding this comment

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

@anntzer

Some notes, possibly for follow-ups:

skew() could also be rewritten to be element-wise
Does it still make sense to keepAffine2D._mtx as a numpy array if we only do element-wise operations?