Integer division

[Antony Lee]

I would like to suggest that division of integers with "/" prints a warning at compile time.  In my opinion, "/" should be restricted to float division, and "//" should be used when rounding semantics are required.

Otherwise, we get weird edge cases such as:

#cython: cdivision=True, infer_types=True
def decl(int m, int n):
    for i in range(1, m):
        for j in range(1, n):
            print(i / j) # integer division

def nodecl(m, n):
    for i in range(1, m):
        for j in range(1, n):
            print(i / j) # float division

(whose behavior may even change if type inference rules changes).

A more extreme choice would be to have "/" always behave like float (double) division even with integer arguments.

Antony

[Robert Bradshaw]

On Mon, Sep 7, 2015 at 11:59 AM, Antony Lee <anton...@berkeley.edu> wrote:
> I would like to suggest that division of integers with "/" prints a warning
> at compile time. In my opinion, "/" should be restricted to float division,
> and "//" should be used when rounding semantics are required.
>
> Otherwise, we get weird edge cases such as:
>
> #cython: cdivision=True, infer_types=True
> def decl(int m, int n):
> for i in range(1, m):
> for j in range(1, n):
> print(i / j) # integer division
>
> def nodecl(m, n):
> for i in range(1, m):
> for j in range(1, n):
> print(i / j) # float division

Slight correction: int or float division depending on whether you're
using Python 2 or Python 3. The runtime type of i/j isn't something we
can delegate to the C compiler.

> (whose behavior may even change if type inference rules changes).
>
> A more extreme choice would be to have "/" always behave like float (double)
> division even with integer arguments.

You mean like putting from __future__ import division at the top of
your file? Or compiling with -3 (for Python 3 language semantics)?

[Antony Lee]

Sorry, I intended the entire discussion to apply to the case of Python3 semantics.  Indeed, even with language_level=3 and from __future__ import division, foo.decl(3, 3) prints "1, 2, 0, 1" whereas foo.nodecl(3, 3) prints "1.0, 0.5, 2.0, 1.0".

Antony

--

---
You received this message because you are subscribed to the Google Groups "cython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cython-users...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Robert Bradshaw]

On Mon, Sep 7, 2015 at 3:29 PM, Antony Lee <anton...@berkeley.edu> wrote:
> Sorry, I intended the entire discussion to apply to the case of Python3
> semantics. Indeed, even with language_level=3 and from __future__ import
> division, foo.decl(3, 3) prints "1, 2, 0, 1" whereas foo.nodecl(3, 3) prints
> "1.0, 0.5, 2.0, 1.0".

from __future__ import division behaves as expected; I just added some
even more explicit tests:
https://github.com/cython/cython/commit/e61da2eb8a292bc34285c895aade523b6d353414

I'm surprised language_level=3 doesn't automatically set this directive.

[Robert Bradshaw]

On Mon, Sep 7, 2015 at 3:46 PM, Robert Bradshaw <robe...@gmail.com> wrote:
> On Mon, Sep 7, 2015 at 3:29 PM, Antony Lee <anton...@berkeley.edu> wrote:
>> Sorry, I intended the entire discussion to apply to the case of Python3
>> semantics. Indeed, even with language_level=3 and from __future__ import
>> division, foo.decl(3, 3) prints "1, 2, 0, 1" whereas foo.nodecl(3, 3) prints
>> "1.0, 0.5, 2.0, 1.0".
>
> from __future__ import division behaves as expected; I just added some
> even more explicit tests:
> https://github.com/cython/cython/commit/e61da2eb8a292bc34285c895aade523b6d353414
>
> I'm surprised language_level=3 doesn't automatically set this directive.

Are you sure you're setting it correctly?
https://github.com/cython/cython/blob/master/tests/run/cython3.pyx#L30

[Antony Lee]

Actually I realized that it's another problem; setting cdivision to True (as I did in the directive of the example I gave, because I basically always set this to True) overrides __future__ division and restores the C division semantic that int / int = int.  The documentation actually says that the cdivision directive should only affect the case of operands with different signs and of division by zero, not whether int / int = int.

Antony

[Robert Bradshaw]

Oh, that's another issue. I'll clarify this in the documentation.

[Robert Bradshaw]

Or perhaps we should respect float division even with cdivision... Thoughts?

[Antony Lee]

My 2c (not cc'ed to cython-devel, as I am not registered):

- language_level=3 implies __future__ division.

- if __future__ division is set: int / int => float (whether variables are untyped (= typed at runtime by python) or typed (including by the inferencer))

- if __future__ division is not set: int / int => int, with opposite-sign-case and zero-divisor-case handled depending on cdivision

- in both cases, int // int => int, with opposite-sign-case and zero-divisor-case handled depending on cdivision (also for % and divmod).

[Robert Bradshaw]

On Mon, Sep 7, 2015 at 5:42 PM, Antony Lee <anton...@berkeley.edu> wrote:
> My 2c (not cc'ed to cython-devel, as I am not registered):
>
> - language_level=3 implies __future__ division.
> - if __future__ division is set: int / int => float (whether variables are
> untyped (= typed at runtime by python) or typed (including by the
> inferencer))
> - if __future__ division is not set: int / int => int, with
> opposite-sign-case and zero-divisor-case handled depending on cdivision
> - in both cases, int // int => int, with opposite-sign-case and
> zero-divisor-case handled depending on cdivision (also for % and divmod).

That's what I'm leaning towards as well, but it is backwards
incompatible. Anyone else have any opinions?

[Ian Henriksen]

Since Cython already tries to make division of C integers behave like Python 

division, I'd expect python-like behavior (mimicking python 2 or 3 as needed) by 

default and unchanged C integer division when the cdivision directive is set.

Best,

-Ian Henriksen

[Chris Barker - NOAA Federal]

>> - if __future__ division is set: int / int => float (whether variables are
>> untyped (= typed at runtime by python) or typed (including by the
>> inferencer))
>> - if __future__ division is not set: int / int => int, with
>> opposite-sign-case and zero-divisor-case handled depending on cdivision
>> - in both cases, int // int => int, with opposite-sign-case and
>> zero-divisor-case handled depending on cdivision (also for % and divmod).
>
> That's what I'm leaning towards as well, but it is backwards
> incompatible.

I like it, but don't think the backwards incompatibility is worth it.

Unless Cython could emit a warning in all cases where the behavior
changed. But even then, it could be pretty annoying.

-Chris

[Antony Lee]

2015-09-08 8:18 GMT-07:00 Chris Barker - NOAA Federal <chris....@noaa.gov>:

>> - if __future__ division is set: int / int => float (whether variables are
>> untyped (= typed at runtime by python) or typed (including by the
>> inferencer))
>> - if __future__ division is not set: int / int => int, with
>> opposite-sign-case and zero-divisor-case handled depending on cdivision
>> - in both cases, int // int => int, with opposite-sign-case and
>> zero-divisor-case handled depending on cdivision (also for % and divmod).
>
> That's what I'm leaning towards as well, but it is backwards
> incompatible.

I like it, but don't think the backwards incompatibility is worth it.

Unless Cython could emit a warning in all cases where the behavior
changed. But even then, it could be pretty annoying.

-Chris

I would consider that language_level=3 not implying __future__ division is a bug.  But fixing it is already backwards incompatible, so I'd just have everything fixed.

I agree that this would necessitate warning on int / int, but IMO Python3 (correctly) decided that there is another operator for integer (rounding) division, namely //.

Antony

[Robert Bradshaw]

On Tue, Sep 8, 2015 at 10:26 AM, Antony Lee <anton...@berkeley.edu> wrote:
>
> 2015-09-08 8:18 GMT-07:00 Chris Barker - NOAA Federal
> <chris....@noaa.gov>:
>>
>> >> - if __future__ division is set: int / int => float (whether variables
>> >> are
>> >> untyped (= typed at runtime by python) or typed (including by the
>> >> inferencer))
>> >> - if __future__ division is not set: int / int => int, with
>> >> opposite-sign-case and zero-divisor-case handled depending on cdivision
>> >> - in both cases, int // int => int, with opposite-sign-case and
>> >> zero-divisor-case handled depending on cdivision (also for % and
>> >> divmod).
>> >
>> > That's what I'm leaning towards as well, but it is backwards
>> > incompatible.
>>
>> I like it, but don't think the backwards incompatibility is worth it.
>>
>> Unless Cython could emit a warning in all cases where the behavior
>> changed. But even then, it could be pretty annoying.
>>
>> -Chris
>>
>
> I would consider that language_level=3 not implying __future__ division is a
> bug. But fixing it is already backwards incompatible, so I'd just have
> everything fixed.

language_level=3 does imply __future__ division, it's just that
cdivision ignores __future__ division (and hence also
language_level=3)

> I agree that this would necessitate warning on int / int, but IMO Python3
> (correctly) decided that there is another operator for integer (rounding)
> division, namely //.

Python 2 has this as well.

[Antony Lee]

2015-09-08 11:04 GMT-07:00 Robert Bradshaw <robe...@gmail.com>:

On Tue, Sep 8, 2015 at 10:26 AM, Antony Lee <anton...@berkeley.edu> wrote:
>
> I would consider that language_level=3 not implying __future__ division is a
> bug.  But fixing it is already backwards incompatible, so I'd just have
> everything fixed.

language_level=3 does imply __future__ division, it's just that
cdivision ignores __future__ division (and hence also
language_level=3)

My bad, I messed up my testing.

[Robert Bradshaw]

No problem. Better to have extra eyeballs on stuff like this than not
enough, so thanks for looking into it :).

[Stefan Behnel]

Antony Lee schrieb am 08.09.2015 um 19:26:
> I would consider that language_level=3 not implying __future__ division is
> a bug. But fixing it is already backwards incompatible

It was already fixed in 0.23. You didn't say which version you tried.

Stefan

[Stefan Behnel]

Robert Bradshaw schrieb am 08.09.2015 um 20:04:
> language_level=3 does imply __future__ division, it's just that
> cdivision ignores __future__ division (and hence also
> language_level=3)

I consider this absolutely reasonable behaviour from a language POV. Future
division is a global Python thing that changes Python semantics for the
module. C declarations and directives usually override Python semantics in
Cython code.

That being said, it would be easier to write readable code if this was not
the case here, given that there is a dedicated integer // operator but no
explicit way to say "but I really want float division for these integer
operands" once you enable C division.

Can't say if it matters in practice.

Stefan

[Chris Barker]

On Tue, Sep 8, 2015 at 12:38 PM, Stefan Behnel <stef...@behnel.de> wrote:

That being said, it would be easier to write readable code if this was not
the case here, given that there is a dedicated integer // operator but no
explicit way to say "but I really want float division for these integer
operands" once you enable C division.

Can't say if it matters in practice.

well, cdivision is mostly (always?) for typed variables, so:

cdef int i,j,k 

i = j/k

has to be integer division anyway.

what does cython do with:

cdef double i

cdef int j, k

i = j / k

it would be nice for that to be floating point division... but again, backward compatibility...

-CHB

 

Stefan

--

---
You received this message because you are subscribed to the Google Groups "cython-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cython-users...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            (206) 526-6959   voice
7600 Sand Point Way NE   (206) 526-6329   fax
Seattle, WA  98115       (206) 526-6317   main reception

Chris....@noaa.gov

[Robert Bradshaw]

On Wed, Sep 9, 2015 at 10:12 AM, Chris Barker <chris....@noaa.gov> wrote:
> On Tue, Sep 8, 2015 at 12:38 PM, Stefan Behnel <stef...@behnel.de> wrote:
>>
>> That being said, it would be easier to write readable code if this was not
>> the case here, given that there is a dedicated integer // operator but no
>> explicit way to say "but I really want float division for these integer
>> operands" once you enable C division.
>>
>> Can't say if it matters in practice.
>
>
> well, cdivision is mostly (always?) for typed variables, so:
>
> cdef int i,j,k
>
> i = j/k
>
> has to be integer division anyway.

What about

cdef int i, j = 2**63 - 1, k = 1
i = j/k

> what does cython do with:
>
> cdef double i
> cdef int j, k
>
> i = j / k
>
> it would be nice for that to be floating point division...

Yes, and I don't want to introduce the notion that the destination
influences expression evaluation.

> but again, backward compatibility...

True, but I don't think it'd be that bad...

- Robert

[Allen B. Riddell]

Some more cents: I hit an issue related to (language level 3) integer
division recently and would prefer breaking backwards compatibility if
it makes behavior more consistent with expectations from Python 3.