Skip to content

np.errstate is not asyncio-safe #9444

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.

Sign up for GitHub

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

Jump to bottom
Closed
eric-wieser opened this issue Jul 20, 2017 · 8 comments · Fixed by #23936
Closed

np.errstate is not asyncio-safe #9444

eric-wieser opened this issue Jul 20, 2017 · 8 comments · Fixed by #23936
Labels
00 - Bug component: numpy._core Project Possible project, may require specific skills and long commitment

Comments

@eric-wieser
Copy link
Member

eric-wieser commented Jul 20, 2017 •
edited
Loading

While it is threadsafe, the with statement gives the false impression that it's always safe. It is not:

import asyncio
import numpy as np

def divide_by_zero():
    np.float64(1) / 0

async def foo():
    for j in range(3):
        with np.errstate(divide='raise'):
            for i in range(2):
                try:
                    divide_by_zero()
                except FloatingPointError:
                    pass
                else:
                    print("foo: Failed to raise!")
                await asyncio.sleep(0.15)

async def bar():
    for j in range(3):
        with np.errstate(divide='ignore'):
            for i in range(2):
                try:
                    divide_by_zero()
                except FloatingPointError:
                    print("bar: raised anyway!")
                await asyncio.sleep(0.11)

loop = asyncio.get_event_loop()
loop.run_until_complete(asyncio.gather(foo(), bar()))
loop.close()

gives:

foo: Failed to raise!
bar: raised anyway!
foo: Failed to raise!

because the error-state is thread-local, but asyncio tasks share a thread.

I don't know if there is any way this can be fixed - it seems like this might be a python bug, and context managers might need an __yield_enter__ and __yield_leave__ mechanism, to be notified when control is leaving the suite via a yield/await
@eric-wieser eric-wieser mentioned this issue Jul 20, 2017
Open
@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Jul 20, 2017 •
edited
Loading

Your solution seems reasonable except that it adds a lot of overhead since you need to call __yield_enter__ on the entire stack of active asynchronous context managers.

@eric-wieser
Copy link
Member Author

Using tasklocals from pypi would also solve the problem

@eric-wieser
Copy link
Member Author

eric-wieser commented Jul 20, 2017 •
edited
Loading

@njsmith is two years ahead of me here with PEP 521, but reassuringly has come to a similar conclusion to how this would need to be fixed

@NeilGirdhar
Copy link
Contributor

Yeah, he even explains the differences between that proposal and the task locals. So, it seems that one day Python will have this sorted out. Until then, there is probably no point in investing time in a "deprecation context manager". Still having a list of what you would deprecate would be really useful to motivate its creation one day?

@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Aug 19, 2017 •
edited by eric-wieser
Loading

FYI: A competing proposal, PEP 550, is being discussed on python-ideas.

@eric-wieser eric-wieser mentioned this issue Oct 28, 2017
Closed
@eric-wieser eric-wieser mentioned this issue Nov 10, 2017
Merged
@eric-wieser eric-wieser mentioned this issue Dec 5, 2017
Open
@eric-wieser eric-wieser mentioned this issue Jan 16, 2018
Merged
@1st1
Copy link

1st1 commented Jan 29, 2018

PEP 567—Context Variables—has been accepted and implemented in Python 3.7. With its new APIs it's now possible to fix this issue.

@NeilGirdhar
Copy link
Contributor

The new np.printoptions should probably be fixed at the same time.

@eric-wieser eric-wieser mentioned this issue Jun 8, 2018
Open
@seberg seberg added the Project Possible project, may require specific skills and long commitment label Nov 10, 2021
@seberg
Copy link
Member

seberg commented Nov 10, 2021

I think this could be attacked now (and would also close gh-19006). Might be nice to see that it doesn't have any performance downsides, but it seemd a bit like contextvars are faster than "TheDict".

This goes way into the depth of the Python C-API, but (as far as I understand right now!) the work used in allocators should outline how this can be achieved: gh-17582

@seberg seberg mentioned this issue Jan 13, 2023
Closed
@seberg seberg mentioned this issue Jun 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
00 - Bug component: numpy._core Project Possible project, may require specific skills and long commitment
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ENH,API: Make the errstate/extobj a contextvar seberg/numpy
4 participants
@seberg @1st1 @eric-wieser @NeilGirdhar