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

ir_utils causes a DeprecationWarning in numpy 1.20 #6175

Closed
Tracked by #6182
eric-wieser opened this issue Aug 27, 2020 · 11 comments · Fixed by #6203
Closed
Tracked by #6182

ir_utils causes a DeprecationWarning in numpy 1.20 #6175

eric-wieser opened this issue Aug 27, 2020 · 11 comments · Fixed by #6203

Comments

@eric-wieser
Copy link
Contributor

eric-wieser commented Aug 27, 2020

These lines:

numba/numba/core/ir_utils.py

Lines 1525 to 1527 in c99f3de

if (hasattr(numpy, value)
and def_val == getattr(numpy, value)):
attrs += ['numpy']

result in the following behavior in numpy 1.20 due to numpy/numpy#14882

In [8]: @numba.njit
   ...: def foo():
   ...:     return bool(1)
   ...:

In [9]: foo()
c:\users\wiese\repos\ga\numba\numba\core\ir_utils.py:1525: DeprecationWarning: `np.bool` is a deprecated alias for the builtin `bool`. Use `bool` by itself, which is identical in behavior, to silence this warning. If you specifically wanted the numpy scalar type, use `np.bool_` here.
  if (hasattr(numpy, value)
c:\users\wiese\repos\ga\numba\numba\core\ir_utils.py:1526: DeprecationWarning: `np.bool` is a deprecated alias for the builtin `bool`. Use `bool` by itself, which is identical in behavior, to silence this warning. If you specifically wanted the numpy scalar type, use `np.bool_` here.
  and def_val == getattr(numpy, value)):
c:\users\wiese\repos\ga\numba\numba\core\ir_utils.py:1525: DeprecationWarning: `np.bool` is a deprecated alias for the builtin `bool`. Use `bool` by itself, which is identical in behavior, to silence this warning. If you specifically wanted the numpy scalar type, use `np.bool_` here.
  if (hasattr(numpy, value)
c:\users\wiese\repos\ga\numba\numba\core\ir_utils.py:1526: DeprecationWarning: `np.bool` is a deprecated alias for the builtin `bool`. Use `bool` by itself, which is identical in behavior, to silence this warning. If you specifically wanted the numpy scalar type, use `np.bool_` here.
  and def_val == getattr(numpy, value)):
Out[9]: True

This doesn't make any sense to me - why is numba even looking at numpy when I use a builtin function?

@gmarkall
Copy link
Member

Thanks for the report - I can see that those lines get hit even when using the builtin.

This doesn't make any sense to me - why is numba even looking at numpy when I use a builtin function?

At this point Numba is still trying to determine what the function and module names are - it happens to try looking at numpy first, and because np.bool simply aliases the builtin bool, it ends up thinking this is a numpy function. However for other builtins, I wouldn't expect the branch

numba/numba/core/ir_utils.py

Lines 1525 to 1527 in c99f3de

if (hasattr(numpy, value)
and def_val == getattr(numpy, value)):
attrs += ['numpy']

to get taken.

@stuartarchibald
Copy link
Contributor

It's probably being run when scanning for closures, reductions and inlinable array calls.

@eric-wieser
Copy link
Contributor Author

Note that it's the hasattr(numpy, value) call that emits the error, so whether the branch is taken is irrelevant.

Can you given me an example of code that does take that branch, and explain why it needs to?

@stuartarchibald
Copy link
Contributor

This would trigger the branch:

from numba import njit
from numpy import array

@njit
def foo(n):
   return array([x for x in range(n)])

foo(4)

the code in question is trying to work out where array comes from. It's seeing if NumPy even has the attribute and if it does then is checking that it's definitely NumPy's array and not something else's array. The purpose in this case is to permit the array-call inliner to reinterpet that array construction as a call to np.empty and then directly assign the values from the range(n) into the array without constructing a list.

@eric-wieser
Copy link
Contributor Author

eric-wieser commented Aug 27, 2020

Does that mean that the following doesn't perform that optimization?

from numba import njit
from numpy import array as not_array

@njit
def foo(n):
   return not_array([x for x in range(n)])

foo(4)

@stuartarchibald
Copy link
Contributor

That'll still get optimised, expr is the global <built-in function array> which has __name__ as 'array' which is picked up here:

numba/numba/core/ir_utils.py

Lines 1507 to 1512 in c99f3de

keys = ['name', '_name', '__name__']
value = None
for key in keys:
if hasattr(callee_def.value, key):
value = getattr(callee_def.value, key)
break

@stuartarchibald
Copy link
Contributor

Been thinking about this a bit, I'm not sure it's avoidable. I think for Numba perhaps it just needs to make sure that internally none of these deprecated aliases are used? If user code is the cause of the deprecation warnings then it's sign the code may need to change? I also think that callee_def.value.__module__ == 'numpy' could be used as a guard to determine whether it's something from NumPy to save tripping this branch for definitions aliasing names in the NumPy attrs?

@eric-wieser
Copy link
Contributor Author

I think for Numba perhaps it just needs to make sure that internally none of these deprecated aliases are used?

Done in #6176

If user code is the cause of the deprecation warnings then it's sign the code may need to change?

The user code at the top of this issue does not reference the deprecated aliases at all. numbas attempt to understand them is causing the problem.

@eric-wieser
Copy link
Contributor Author

I also think that callee_def.value.__module__ == 'numpy' could be used as a guard to determine whether it's something from NumPy to save tripping this branch for definitions aliasing names in the NumPy attrs?

That seems quite reasonable to me

@stuartarchibald
Copy link
Contributor

I also think that callee_def.value.__module__ == 'numpy' could be used as a guard to determine whether it's something from NumPy to save tripping this branch for definitions aliasing names in the NumPy attrs?

That seems quite reasonable to me

If this is done, I think this applies:

If user code is the cause of the deprecation warnings then it's sign the code may need to change?

Numba would only be asking NumPy about things that are likely actually in NumPy and then if one of these things is deprecated and NumPy warns, that's legitimate?

@eric-wieser
Copy link
Contributor Author

Yes, sounds good to me.

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.

3 participants