Skip to content
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

call to colorbar not thread safe #1889

Closed
apatlpo opened this issue Feb 6, 2018 · 12 comments
Closed

call to colorbar not thread safe #1889

apatlpo opened this issue Feb 6, 2018 · 12 comments

Comments

@apatlpo
Copy link
Contributor

apatlpo commented Feb 6, 2018

The following call in xarray/xarray/plot/plot.py does not seem to be thread safe:

            cbar = plt.colorbar(primitive, **cbar_kwargs)

It leads to systematic crashes when distributed, with a cryptic error message (ValueError: Unknown element o). I have to call colorbars outside the xarray plot call to prevent crashes.

A call of the following type may fix the problem:

            cbar = fig.colorbar(primitive, **cbar_kwargs)

But fig does not seem to be available directly in plot.py. Maybe:

            cbar = ax.get_figure().colorbar(primitive, **cbar_kwargs)

cheers

@fmaussion
Copy link
Member

Thanks for the report! Would you like to open a PR? I think you are the best placed to see if you proposed fixes solve the problem for you

@apatlpo
Copy link
Contributor Author

apatlpo commented Feb 6, 2018

Hi,

Yes why not even though I am not too familiar with the process.

I am not even able to properly install the library so far ... python setup.py install creates the following library: /home1/datahome/aponte/.miniconda3/envs/pangeo/lib/python3.6/site-packages/xarray-0.10.0rc1_2_gf83361c-py3.6.egg/
I am surely doing something wrong, I'd like to have xarray.egg.

@fmaussion
Copy link
Member

You are lucky, we just added a contributing guide to xarray: http://xarray.pydata.org/en/latest/contributing.html ;-)

The recommended way to install xarray (and any other local library btw) is:

pip install -e .

@apatlpo
Copy link
Contributor Author

apatlpo commented Feb 6, 2018

thanks, got it.
I tried the suggested fix but it did not work.
Unfortunately I don't have more time on the subject for a couple of days. I'll keep you posted when I can sort things out.

@shoyer
Copy link
Member

shoyer commented Feb 6, 2018

I agree that it is certainly a best practice to avoid relying on global state from the pyplot module.

That said, my experience has been that even if you avoid using pyplot functions matplotlib is still not thread-safe (StackOverflow backs me up here). So you probably will need a thread-lock around your use of any plotting functions, anyways.

@apatlpo
Copy link
Contributor Author

apatlpo commented Feb 9, 2018

Ok, thanks, do you have a piece of code with a thread-lock that I could get inspiration from?

@shoyer
Copy link
Member

shoyer commented Feb 9, 2018

@apatlpo sure, something like:

import threading
import matplotlib.pyplot

MPL_LOCK = threading.Lock()

def save_imshow(data, path):
    with MPL_LOCK:
        fig, ax = plt.subplot()
        ax.imshow(data)
        fig.savefig(path)

save_imshow() is now something you safely call in parallel (e.g., from dask.array.map_blocks()).

@jhamman
Copy link
Member

jhamman commented Feb 26, 2018

Closing for now. @apatlpo, feel free to reopen if there's more to discuss here.

@jhamman jhamman closed this as completed Feb 26, 2018
@apatlpo
Copy link
Contributor Author

apatlpo commented Apr 7, 2020

I reopen this issue as it came across my road again when generating figures on a dask.Distributed LocalCluster.
I just open a PR suggesting a change that solves the issue in my situation.

@apatlpo
Copy link
Contributor Author

apatlpo commented Apr 7, 2020

hehe. I cannot reopen the issue apparently

@dcherian
Copy link
Contributor

dcherian commented Sep 4, 2020

Another way to do this would be to use da.map_blocks(plotfunc, template=...)
with

def plotfunc(da):
    f = plt.figure()
    ...
    f.close()

    return da.time  # or similar

I have used this successfully with a distributed cluster.

@stale
Copy link

stale bot commented Apr 27, 2022

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

@stale stale bot added the stale label Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants