Improve speed in some modules - #22678
Conversation
0f9decf to
295bef0
Compare
295bef0 to
7b8fd5e
Compare
|
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 ;-) |
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).