FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Don't call np.identity() in transforms. by anntzer · Pull Request #10327 · matplotlib/matplotlib · GitHub

Repository navigation

Don't call np.identity() in transforms. - #10327

Merged
jklymak merged 1 commit into
matplotlib:masterfrom
anntzer:fasttransform
Jan 29, 2018
Merged

jklymak merged 1 commit into
matplotlib:masterfrom
anntzer:fasttransform

Conversation

anntzer commented Jan 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

EDIT: see below.

Speeds up setting up of subplots by ~3% and their drawing by ~2% at a
reasonably limited cost in legibility.

The difference is actually not easy to observe because variation in
timings are much greater than 3% from run to run.

In a call like subplots(10, 10), just setting up the figure calls
np.identity(3) 1801 times and actually drawing the figure another 1705
times (measured by manually instrumenting the calls to identity() in the
old version. At the same time,

%timeit np.identity(3)

and

%%timeit t = np.identity(3)
t.copy()

show that on my machine, a call to np.identity(3) is approximately 16us
slower than copying a preexisting matrix; multiplying this by 1801 shows
that the calls to identity correspond to an excess time of ~27ms.

Finally, setting up the axes and drawing it can be timed using

from time import perf_counter
import matplotlib; matplotlib.use("agg")
from matplotlib import pyplot as plt

N = 10
 # First draw always seems slower, so skip it.
figure = plt.figure()
figure.subplots(N, N)
figure.canvas.draw()
plt.close("all")

for _ in range(5):
    figure = plt.figure()
    start = perf_counter()
    figure.subplots(N, N)
    print("{: 4}".format(int(1000 * (perf_counter() - start))), end=" ")
    start = perf_counter()
    figure.canvas.draw()
    print("{: 4}".format(int(1000 * (perf_counter() - start))))
    plt.close("all")

which shows that (on my machine) setting up the subplots takes ~850ms
and drawing them ~1300ms (but again, with a lot of jitter).

Hence, the gain from the patch should be ~3% for the setup and ~2% for
the draw.


Edit: I realized that my timings were invalid because I had https://pypi.org/project/ipython-autoimport/ active which, while reasonably fast, still messed up the timings a bit.
The real gain is probably ~5x less than what advertised above, so ~0.5%. Feel free to close if you think this is not worth it.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

jklymak commented Jan 26, 2018

Copy link
Copy Markdown
Member

We are getting odd failures on 2.7 with some of your PRs now. Not sure if it is "you" or "us"...

This change seems fine to me.

anntzer commented Jan 26, 2018

Copy link
Copy Markdown
Contributor Author

Yeah, I've noticed failures from this specific test (which seems unrelated)...

jklymak added this to the v2.2 milestone Jan 26, 2018
Speeds up setting up of subplots by ~3% and their drawing by ~2% at a
reasonably limited cost in legibility.

The difference is actually not easy to observe because variation in
timings are much greater than 3% from run to run.

In a call like `subplots(10, 10)`, just setting up the figure calls
`np.identity(3)` 1801 times and actually drawing the figure another 1705
times (measured by manually instrumenting the calls to identity() in the
old version.  At the same time,

```
%timeit np.identity(3)
```
and
```
%%timeit t = np.identity(3)
t.copy()
```
show that on my machine, a call to np.identity(3) is approximately 16us
slower than copying a preexisting matrix; multiplying this by 1801 shows
that the calls to identity correspond to an excess time of ~27ms.

Finally, setting up the axes and drawing it can be timed using
```
from time import perf_counter
import matplotlib; matplotlib.use("agg")
from matplotlib import pyplot as plt

N = 10
 # First draw always seems slower, so skip it.
figure = plt.figure()
figure.subplots(N, N)
figure.canvas.draw()
plt.close("all")

for _ in range(5):
    figure = plt.figure()
    start = perf_counter()
    figure.subplots(N, N)
    print("{: 4}".format(int(1000 * (perf_counter() - start))), end=" ")
    start = perf_counter()
    figure.canvas.draw()
    print("{: 4}".format(int(1000 * (perf_counter() - start))))
    plt.close("all")
```
which shows that (on my machine) setting up the subplots takes ~850ms
and drawing them ~1300ms (but again, with a lot of jitter).

Hence, the gain from the patch should be ~3% for the setup and ~2% for
the draw.

dstansby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👍 nice! Feel free to merge once the 2.7 travis build passes

jklymak merged commit 6c4fd69 into matplotlib:master Jan 29, 2018
anntzer deleted the fasttransform branch January 29, 2018 19:16
QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL