Skip to content

Python semantics for mod of negative int #365

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
jbrockmendel opened this issue Sep 18, 2020 · 3 comments
Closed

Python semantics for mod of negative int #365

jbrockmendel opened this issue Sep 18, 2020 · 3 comments
Labels

Comments

@jbrockmendel
Copy link

arr = np.array([-2, -2])
result = ne.evaluate("arr % 4")
expected = arr % 4

>>> result
array([-2, -2], dtype=int64)
>>> expected
array([2, 2])

AFAICT ne.evaluate is using C semantics for divmod, rounding the div towards zero instead of towards -inf. It would be nice if we could enable python semantics here, analogous to @cython.cdivision(False)

@robbmcleod
Copy link
Member

Well it's not practical to add new op-codes in version 2.7, as they're all used up. So it would imply changing the behavior of NumExpr to use the sign of the divisor to remain consistent in the Python eco-system. I'm inclined to change it because NumExpr isn't consistent, if you use literals,

ne.evaluate('-2 % 4')

the result is the expected Python value 2. If you using floating-point, it also works how Python does:

arr = np.array([-2, -2], dtype=np.float32)
result = ne.evaluate("arr % 4")
expected = arr % 4

>>> result
array([2., 2.], dtype=float32)
>>> expected
array([2., 2.], dtype=float32)

The actual functions are as follows; for integer mod (from numexpr/interp_body.cpp):

case OP_MOD_LLL: VEC_ARG2(l_dest = l2 ? (l1 % l2) : 0);

and for floating-point mod:

case OP_MOD_DDD: VEC_ARG2(d_dest = d1 - floor(d1/d2) * d2);

So basically I need a macro for copysign that works for ints. One possible implementation:

#define COPYSIGN_INT(X, Y) (Y > 0 ? (X < 0 ? -X : X) : (X < 0 ? X : -X))

case OP_MOD_LLL: VEC_ARG2(l_dest = COPYSIGN_INT(l2 ? (l1 % l2) : 0, l2));

This is ugly, there's probably a faster implementation out there somewhere.

@robbmcleod
Copy link
Member

The implementation of copysign for double makes use of the sign bit:

http://www.netlib.org/fdlibm/s_copysign.c

I'm guessing there's some slick way to do it with bit-shift operations for integers. NumPy's implementation appears to be rather naive:

https://github.com/numpy/numpy/edit/master/numpy/core/src/npymath/npy_math_internal.h.src

Copy link

Message to comment on stale issues. If none provided, will not mark issues stale

@github-actions github-actions bot added the Stale label Feb 18, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants