Skip to content

[Bug]: slow rendering of multiple axes (time scales as 2nd power of label count) #21895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
FilipDominec opened this issue Dec 9, 2021 · 6 comments · May be fixed by #21958
Closed

[Bug]: slow rendering of multiple axes (time scales as 2nd power of label count) #21895

FilipDominec opened this issue Dec 9, 2021 · 6 comments · May be fixed by #21958

Comments

@FilipDominec
Copy link

Bug summary

I need to label a lot of elemental emission peaks on X-ray spectra. There are three series of peaks denoted K, L and M, so I put three axes above the plot. (This is the best approach I could find online, it does not seem that multiple text labels would be covered in the docs. I will be thankful for any tip.)

Anyway, the plotting then goes very very slow: roughly 4× slower when the number of labels doubles.

Code for reproduction

#!/usr/bin/python3  
#-*- coding: utf-8 -*-

## Import common moduli
import matplotlib, sys
matplotlib.use("cairo") # helps a bit
#matplotlib.rc('text', usetex=False)
import matplotlib.pyplot as plt
import numpy as np

fig, ax = plt.subplots(nrows=1, ncols=1, figsize=(16, 10))
x,y = [1,2,3], [5,6,4] ## dummy data
#x,y = np.
ax.plot(x,y, marker='o')

elem = ["H", "He", "Li", "Be", "B", "C", "N", "O", "F", "Ne",  "Na", "Mg", "Al", "Si", "P", "S", "Cl", "Ar", "K", "Ca", "Sc", "Ti", "V", "Cr", "Mn", "Fe", "Co", "Ni", "Cu", "Zn", "Ga", "Ge", "As", "Se", "Br", "Kr", "Rb", "Sr", "Y", "Zr", "Nb", "Mo", "Tc", "Ru", "Rh", "Pd", "Ag", "Cd", "In", "Sn", "Sb", "Te", "I", "Xe", "Cs", "Ba", "La", "Ce", "Pr", "Nd", "Pm", "Sm", "Eu", "Gd", "Tb", "Dy", "Ho", "Er", "Tm", "Yb", "Lu", "Hf", "Ta", "W", "Re", "Os", "Ir", "Pt", "Au", "Hg", "Tl", "Pb", "Bi", "Po", "At", "Rn", "Fr", "Ra", "Ac", "Th", "Pa", "U", "Np", "Pu", "Am", "Cm", "Bk", "Cf", "Es"][:int(sys.argv[1]) if len(sys.argv)>1 else 1000]


max_x = 14
use_math = False

if max_x: ax.set_xlim(xmin=0, xmax=max_x)

posK = np.arange(len(elem))**2 *3/4*0.0136 # a bit inaccurate Moseley law for K-series
ax2 = ax.twiny()
ax2.plot(x,y, color="None")
ax2.set_xticks(posK)  # (must precede set_xlim)
if use_math:
    ax2.set_xticklabels((s+"$_K$" for s in elem), fontsize=8) 
else:
    ax2.set_xticklabels((s+"K" for s in elem), fontsize=8)  # helps a bit
if max_x: ax2.set_xlim(xmin=0, xmax=max_x)

posL = np.arange(len(elem))**2 *3/4*0.0136 / 3.9 # very inaccurate Moseley law for L-series
ax3 = ax.twiny()
ax3.plot(x,y, color="None")
ax3.set_xticks(posL) 
if use_math:
    ax3.set_xticklabels((s+"$_L$" for s in elem), fontsize=8) 
else:
    ax3.set_xticklabels((s+"_L" for s in elem), fontsize=8) 
if max_x: ax3.set_xlim(xmin=0, xmax=max_x)
ax3.spines["top"].set_position(("axes", 1.05))

posM = np.arange(len(elem))**2 *3/4*0.0136 / 9.9 # very inaccurate Moseley law for M-series
ax4 = ax.twiny()
ax4.plot(x,y, color="None")
ax4.set_xticks(posM) 
if use_math:
    ax4.set_xticklabels((s+"$_M$" for s in elem), fontsize=8) 
else:
    ax4.set_xticklabels((s+"_M" for s in elem), fontsize=8) 
if max_x: ax4.set_xlim(xmin=0, xmax=max_x)
ax4.spines["top"].set_position(("axes", 1.10))

print('ready to output', flush=True)
fig.savefig("output.png", bbox_inches='tight')

Actual outcome

By default QtAgg is used. With use_math=True it takes 1m59,698s, without it only 0m18,759s.

When I switch backend to cairo: With use_math=True it takes 1m30,385s, without it only 0m11,522s. Still a lot.

output

Expected outcome

Such a simple plot could render within few seconds, even if lower indices are used rendered using the $ ... $ notation.

Also, as a separate minor bug, I can see fontsize=8 has applied on the first extra axis only. The other two would not change their font size even when explicitly asked for.

Additional information

This bug was observed also in Matplotlib 3.1.x taken from the Ubuntu repository. Today I checked out the devel version and it is basically the same.

Almost all the code runs within a second. The slow command is the savefig.

I don't know why this happens, it looks like if every next label somehow needed to "update" all existing ones; this would explain the quadratic dependence of time.

I don't know any fix.

Operating system

Ubuntu

Matplotlib Version

3.6.0.dev995+g21b76ae7f7

Matplotlib Backend

QtAgg by default,

Python version

Python 3.8.10 (default, Sep 28 2021, 16:10:42)

Jupyter version

N/A

Installation

git checkout

@anntzer
Copy link
Contributor

anntzer commented Dec 9, 2021

I can confirm the issue. This occurs because text layouting is somewhat slow, but most of the code assumes it's cheap because we have caches at

_cached = cbook.maxdict(50)
and
@functools.lru_cache(50)

but unfortunately you have more ticklabels than the cache and the strings get repeatedly reparsed. Indeed, increasing the size of either cache to 10000 fixes the slowness (the mathtext cache only affects the use_math case, of course).

I guess(?) a fix is to move the Text._cached cache info to a private attribute on each Text instance, so that each instance caches its own info and at least we don't get limited artificially by the arbitrary cache size.

@tacaswell tacaswell added this to the v3.6.0 milestone Dec 9, 2021
@tacaswell tacaswell added the Good first issue Open a pull request against these issues if there are no active ones! label Dec 9, 2021
@tacaswell
Copy link
Member

I am 👍🏻 on moving the state caching into the Text objects. The cache cost natural scales with the amount of text and while LRU caches are nice with pure-functions (as the call sights do not need to be aware of the cache), in the case we already have objects and state floating around (that _cached dictionary is a class attribute on Text).


I'm labeling this as a good first issue as it does not involve any API design choices, but it will need an intermediate understanding of Python as you will need to chase through the code to where the cached functions are called from and hoist the cache management up a level (or three).

Timing tests are notoriously hard to get right on CI (because we are running on free CPU on the cloud there is a lot of variability and we get preempted in favor of paying customers (which is quite fair!)) and between machines. Maybe compare the render times of a figure with lots of math text and one with no text? If we want to verify that the caching helps, maybe verify that the first draw is slower than the second? That is a long way of saying, a PR with a performance regression test is better, but I would not block merging over it.

@ojeda-e
Copy link
Contributor

ojeda-e commented Dec 10, 2021

Can I work on this issue @tacaswell @anntzer? :)

@anntzer
Copy link
Contributor

anntzer commented Dec 10, 2021

Go ahead, I agree with @tacaswell's assessment on the issue.
(See also note at https://matplotlib.org/devdocs/devel/contributing.html#issues-for-new-contributors re: issue assignment.)

There's a somewhat similar cache on images (_imcache and _rgbacache) which may or may not be worth comparing with to understand the caching here.

@anntzer
Copy link
Contributor

anntzer commented Jul 11, 2022

AFAICT this has been fixed by #22271, please confirm?

@tacaswell tacaswell added status: needs confirmation and removed Good first issue Open a pull request against these issues if there are no active ones! labels Sep 1, 2022
@anntzer
Copy link
Contributor

anntzer commented Jan 21, 2023

Closing as tentatively fixed by #22271.

@anntzer anntzer closed this as completed Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants