Conversation
oscargus
force-pushed
the
speedimprovements
branch
from
0f9decf to
295bef0
Compare
oscargus
force-pushed
the
speedimprovements
branch
from
295bef0 to
7b8fd5e
Compare
|
I'm actually quite surprised (but remember being surprised by this before) that raising to a fraction power is pretty fast: $ ipython
Python 3.11.0a6+ (heads/main:29624e769c, Mar 18 2022, 18:36:12) [GCC 11.2.0]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.2.0.dev -- An enhanced Interactive Python. Type '?' for help.
In [1]: import numpy as np
In [2]: %timeit np.sqrt(52)
476 ns ± 1.14 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
In [3]: %timeit 52 ** (1/2)
3.12 ns ± 0.018 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [4]: import math
In [5]: %timeit math.sqrt(52)
25.4 ns ± 0.125 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)
In [6]: s = math.sqrt
In [7]: %timeit s(52)
25.4 ns ± 0.0733 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)
In [8]: %timeit s(52.0)
14.9 ns ± 0.0463 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [9]: %timeit 52.0 ** (1/2)
3.15 ns ± 0.00981 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)
In [10]: |
|
Speaking of, can we write them as |
| # determine 95% confidence intervals of the median | ||
| M = len(data) | ||
| percentiles = [2.5, 97.5] | ||
| percentiles = (2.5, 97.5) |
There was a problem hiding this comment.
I don't think this matters wrt speed (does it?); also I prefer a list here as this matches "more" the return type of np.percentile below (a ndarray is closer to a list than to a tuple.)
| # medians and quartiles | ||
| q1, med, q3 = np.percentile(x, [25, 50, 75]) | ||
| percentiles = (25, 50, 75) | ||
| q1, med, q3 = np.percentile(x, percentiles) |
| N = len(data) | ||
| notch_min = med - 1.57 * iqr / np.sqrt(N) | ||
| notch_max = med + 1.57 * iqr / np.sqrt(N) | ||
| half_height = 1.57 * iqr / (N ** (1 / 2)) |
There was a problem hiding this comment.
I guess you could inline len(data) into the computation here.
| MAGICSUM = SQRTHALF + MAGIC45 | ||
| MAGICDIFF = SQRTHALF - MAGIC45 | ||
|
|
||
| vertices = np.array([[0.0, -1.0], |
There was a problem hiding this comment.
I guess I would just write this is as vertices = np.array([...]) without even bothering with the dtype=float which is unneeded; this also avoids a temporary variable (mostly avoids having to keep track of it mentally). Ditto below.
| else: | ||
| verts = polypath.vertices | ||
| y = (1 + np.sqrt(5)) / 4. | ||
| y = (1 + 5 ** (1 / 2)) / 4. |
There was a problem hiding this comment.
I'm not convinced of all these sqrt micro-optimizations. IMHO it makes the code less readable, and the performance gain is minimal. For example here
The sqrt is less than 2% of the the time of the immediate surrounding code in the function:
In [21]: %%timeit
...: Affine2D().scale(0.5)
...: polypath = Path.unit_regular_polygon(5)
...: verts = polypath.vertices
...: top = Path(verts[[0, 1, 4, 0]])
...: bottom = Path(verts[[1, 2, 3, 4, 1]])
...: left = Path([verts[0], verts[1], verts[2], [0, -y], verts[0]])
...: right = Path([verts[0], verts[4], verts[3], [0, -y], verts[0]])
...:
61.1 µs ± 228 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
In [22]: %timeit np.sqrt(5)
2.36 µs ± 35 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
and that function _set_pentagon() again is only used to create a new marker object, which in itself is quite rare. So I bet the performance gain is not measurable in any real example.
|
@oscargus Thanks for trying to speed the code up, However, as said before:
Unless we can show there is a measurable performance gain in real situations, I propose to close this. |
|
@oscargus is it OK to close this? I agree that some of the optimizations are marginal at best... In either case, moving to draft ;-) |
|
Closing because I don't think this will give a measureable preformance benefit. |
7 participants
PR Summary
Enable pre-computation during compilation and remove redundant computations.
Before:
After:
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).