Skip to content

Commit

Permalink
FIX: Transition scheme for safer in-place ufunc operations
Browse files Browse the repository at this point in the history
In numpy 1.6 and earlier, if you do
  np.add(int_arr, float_arr, out=int_arr)
or
  int_arr += float_arr
then the result will be silently truncated to integer values. This
often produces bugs, because it's easy to accidentally end up with an
integer array and not realize it.

Therefore, there seems to be consensus that we should switch to using
same_kind casting by default for in-place ufunc operations. However,
just switching this (as was done initially during the 1.7 development
cycle) breaks a lot of code, which is rude and violates our
deprecation policy.

This commit instead adds a special temporary casting rule which acts
like "unsafe", but also checks whether each operation would be allowed
under "same_kind" rules and issues a DeprecationWarning if not.

It also moves NPY_DEFAULT_ASSIGN_CASTING into the formal API instead
of leaving it as a #define. This way we can change it later, and any
code which references it and is compiled against this version of numpy
will automatically switch to whatever we change it too. This avoids
the situation where we want to remove the temporary magic value we're
using to create DeprecationWarnings now, but can't because it would be
an ABI break.
  • Loading branch information
njsmith committed Sep 20, 2012
1 parent 4708615 commit cea0a20
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 8 deletions.
12 changes: 8 additions & 4 deletions doc/release/1.7.0-notes.rst
Expand Up @@ -33,10 +33,14 @@ np.diagonal, numpy 1.7 produces a FutureWarning if it detects
that you may be attemping to write to such an array. See the documentation
for array indexing for details.

The default casting rule for UFunc out= parameters has been changed from
'unsafe' to 'same_kind'. Most usages which violate the 'same_kind'
rule are likely bugs, so this change may expose previously undetected
errors in projects that depend on NumPy.
In a future version of numpy, the default casting rule for UFunc out=
parameters will be changed from 'unsafe' to 'same_kind'. (This also
applies to in-place operations like a += b, which is equivalent to
np.add(a, b, out=a).) Most usages which violate the 'same_kind' rule
are likely bugs, so this change may expose previously undetected
errors in projects that depend on NumPy. In this version of numpy,
such usages will continue to succeed, but will raise a
DeprecationWarning.

Full-array boolean indexing has been optimized to use a different,
optimized code path. This code path should produce the same results,
Expand Down
6 changes: 6 additions & 0 deletions doc/source/reference/ufuncs.rst
Expand Up @@ -309,6 +309,12 @@ advanced usage and will not typically be used.
'equiv', 'safe', 'same_kind', or 'unsafe'. See :func:`can_cast` for
explanations of the parameter values.

In a future version of numpy, this argument will default to
'same_kind'. As part of this transition, starting in version 1.7,
ufuncs will produce a DeprecationWarning for calls which are
allowed under the 'unsafe' rules, but not under the 'same_kind'
rules.

*order*

.. versionadded:: 1.6
Expand Down
2 changes: 2 additions & 0 deletions numpy/core/code_generators/numpy_api.py
Expand Up @@ -14,10 +14,12 @@

multiarray_global_vars = {
'NPY_NUMUSERTYPES': 7,
'NPY_DEFAULT_ASSIGN_CASTING': 292,
}

multiarray_global_vars_types = {
'NPY_NUMUSERTYPES': 'int',
'NPY_DEFAULT_ASSIGN_CASTING': 'NPY_CASTING',
}

multiarray_scalar_bool_values = {
Expand Down
9 changes: 5 additions & 4 deletions numpy/core/include/numpy/ndarraytypes.h
Expand Up @@ -199,11 +199,12 @@ typedef enum {
/* Allow safe casts or casts within the same kind */
NPY_SAME_KIND_CASTING=3,
/* Allow any casts */
NPY_UNSAFE_CASTING=4
} NPY_CASTING;
NPY_UNSAFE_CASTING=4,

/* The default casting to use for typical assignment operations */
#define NPY_DEFAULT_ASSIGN_CASTING NPY_SAME_KIND_CASTING
/* Temporary internal definition only, will be removed in upcoming
release, see below */
NPY_INTERNAL_UNSAFE_CASTING_BUT_WARN_UNLESS_SAME_KIND = 100,
} NPY_CASTING;

typedef enum {
NPY_CLIP=0,
Expand Down
14 changes: 14 additions & 0 deletions numpy/core/src/multiarray/common.c
Expand Up @@ -13,6 +13,20 @@
#include "common.h"
#include "buffer.h"

/* The casting to use for implicit assignment operations resulting from
* in-place operations (like +=) and out= arguments. (Notice that this
* variable is misnamed, but it's part of the public API so I'm not sure we
* can just change it. Maybe someone should try and see if anyone notices.
*/
/* In numpy 1.6 and earlier, this was NPY_UNSAFE_CASTING. In a future
* release, it will become NPY_SAME_KIND_CASTING. Right now, during the
* transitional period, we continue to follow the NPY_UNSAFE_CASTING rules (to
* avoid breaking people's code), but we also check for whether the cast would
* be allowed under the NPY_SAME_KIND_CASTING rules, and if not we issue a
* warning (that people's code will be broken in a future release.)
*/
NPY_NO_EXPORT NPY_CASTING NPY_DEFAULT_ASSIGN_CASTING = NPY_INTERNAL_UNSAFE_CASTING_BUT_WARN_UNLESS_SAME_KIND;


NPY_NO_EXPORT PyArray_Descr *
_array_find_python_scalar_type(PyObject *op)
Expand Down
29 changes: 29 additions & 0 deletions numpy/core/src/multiarray/convert_datatype.c
Expand Up @@ -503,12 +503,41 @@ type_num_unsigned_to_signed(int type_num)
}
}

/* NOTE: once the UNSAFE_CASTING -> SAME_KIND_CASTING transition is over,
* we should remove NPY_INTERNAL_UNSAFE_CASTING_BUT_WARN_UNLESS_SAME_KIND
* and PyArray_CanCastTypeTo_impl should be renamed back to
* PyArray_CanCastTypeTo.
*/
static npy_bool
PyArray_CanCastTypeTo_impl(PyArray_Descr *from, PyArray_Descr *to,
NPY_CASTING casting);

/*NUMPY_API
* Returns true if data of type 'from' may be cast to data of type
* 'to' according to the rule 'casting'.
*/
NPY_NO_EXPORT npy_bool
PyArray_CanCastTypeTo(PyArray_Descr *from, PyArray_Descr *to,
NPY_CASTING casting)
{
if (casting == NPY_INTERNAL_UNSAFE_CASTING_BUT_WARN_UNLESS_SAME_KIND) {
npy_bool unsafe_ok, same_kind_ok;
unsafe_ok = PyArray_CanCastTypeTo_impl(from, to, NPY_UNSAFE_CASTING);
same_kind_ok = PyArray_CanCastTypeTo_impl(from, to,
NPY_SAME_KIND_CASTING);
if (unsafe_ok && !same_kind_ok) {
DEPRECATE("Implicitly casting between incompatible kinds. In "
"a future numpy release, this will become an error. "
"Use casting=\"unsafe\" if this is intentional.");
}
return unsafe_ok;
} else {
return PyArray_CanCastTypeTo_impl(from, to, casting);
}
}

static npy_bool
PyArray_CanCastTypeTo_impl(PyArray_Descr *from, PyArray_Descr *to,
NPY_CASTING casting)
{
/* If unsafe casts are allowed */
Expand Down
18 changes: 18 additions & 0 deletions numpy/core/tests/test_ufunc.py
Expand Up @@ -742,5 +742,23 @@ def t(expect, func, n, m):
uf.accumulate(np.zeros((30, 30)), axis=0)
uf.accumulate(np.zeros((0, 0)), axis=0)

def test_safe_casting(self):
# In old numpy's, any casting was allowed for in-place operations. In
# future numpy's, only same_kind casting will be allowed by
# default.
a = np.array([1, 2, 3], dtype=int)
# Non-in-place addition is fine
assert_array_equal(assert_no_warnings(np.add, a, 1.1),
[2.1, 3.1, 4.1])
assert_warns(DeprecationWarning, np.add, a, 1.1, out=a)
assert_array_equal(a, [2, 3, 4])
def add_inplace(a, b):
a += b
assert_warns(DeprecationWarning, add_inplace, a, 1.1)
assert_array_equal(a, [3, 4, 5])
# Make sure that explicitly overriding the warning is allowed:
assert_no_warnings(np.add, a, 1.1, out=a, casting="unsafe")
assert_array_equal(a, [4, 5, 6])

if __name__ == "__main__":
run_module_suite()

0 comments on commit cea0a20

Please sign in to comment.