PERF: Speed up log and symlog scale transforms #30993
Conversation
d6f1a28 to
49dec48
Compare
49dec48 to
4414066
Compare
|
|
||
| @image_comparison(["test_loglog_nonpos.png"], remove_text=True, style='mpl20', | ||
| tol=0 if platform.machine() == 'x86_64' else 0.029) | ||
| tol=0.029) |
There was a problem hiding this comment.
Why do we need to increated the tol for x86 now?
There was a problem hiding this comment.
There was a 0.006 RMS change in the image test on some platforms, I chalked it up to round-off
lib/matplotlib/scale.py
Outdated
| self.base = base | ||
| self._clip = _api.check_getitem( | ||
| {"clip": True, "mask": False}, nonpositive=nonpositive) | ||
| self._log_func = {np.e: np.log, 2: np.log2, 10: np.log10}.get(base) |
There was a problem hiding this comment.
This introduces state depending on base. While highly unlikely, this would break user code that has changed the base. Are we ok with this?
We possibly should make base private (rename base -> _base) and re-expose baseas a property. You can then decide whether you leave out the setter - with the argument that nobody uses it anyway; or implement it with updateing_log_func`.
Anyway, erroring out on a missing setter would still be better than accepting a new base, but continuing to use the old log function.
Same for some other transforms below.
There was a problem hiding this comment.
Yeah, that's a really good call.
I did a github search and found a few instances of code setting these, so added those as properties with setters. Also deprecated invlinthresh since it's a derived attr and I couldn't find any instances of setting that.
Speedups are unchanged after this.
PR summary
This speeds up log and symlog scale transforms by precomputing some constants, putting in a fast path for symlog when all the points are in the linear region, using specific exponential functions where available, and switching from the slow
np.power(base, x)to the fasternp.exp(x * np.log(base)).On a comparison benchmark of 100000 points, I'm seeing the following speedup factors with the new vs old algorithms:
PR checklist