PERF: reduce startup overhead from cmap registration #4538
Conversation
neutrinoceros
force-pushed
the
faster_cmaps
branch
from
f6ee3a3
to
857f300
Compare
neutrinoceros
force-pushed
the
faster_cmaps
branch
2 times, most recently
from
2b5153e
to
24ad119
Compare
| from matplotlib.colors import LinearSegmentedColormap | ||
|
|
||
| issue_deprecation_warning( | ||
| "yt.visualization.color_maps.add_colormap is deprecated", |
There was a problem hiding this comment.
what is it being replaced with?
There was a problem hiding this comment.
nothing, it's not strictly needed !
| try: | ||
| add_colormap(k, cdict) | ||
| mpl.colormaps.register(ListedColormap(v, name=k, N=256)) |
There was a problem hiding this comment.
I mean, this is slightly flatter, but I don't think it's a big deal, either.
There was a problem hiding this comment.
yeah, this PR yields disappointing results, but just in case, I want to clarify that the gain here is not expected to come from the inlining, but from instantiating ListedColormap instead of LinearSegmentedColormap, which saves a conversion from the format we actually define our maps in.
| cmap.name = hist_name | ||
| mpl.colormaps.register(cmap) | ||
| mpl.colormaps.register(cmap.reversed()) | ||
| cmap = mpl.colormaps[alias] |
There was a problem hiding this comment.
note for later that this even if we close this PR, this line change is worth using if only because mpl.colormaps.__getitem__ already returns copies, so the current implementation of this line is unjustified.
neutrinoceros
force-pushed
the
faster_cmaps
branch
from
24ad119
to
b87982f
Compare
|
I'll drop this (again) because it's far too big a patch for such a small gain. I'll reopen a much smaller PR with just the simplest avoid-double-copies patch (which impacts 2 lines) |
PR Summary
another, maybe more aggressive attempt at minimising startup overhead from colormaps registration (2nd take following #4520)