Opened 14 years ago

Closed 14 years ago

#3571 closed defect (fixed)

[with patch, positive review] ivalue field in integer_mod.pyx shouldn't be public

Reported by: craigcitro Owned by: craigcitro
Priority: major Milestone: sage-3.3
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The ivalue field for IntegerMod_int is public, but it shouldn't be. The following is very frightening, for instance:

sage: R = Integers(10)
sage: x = R(2)
sage: x
2
sage: x.ivalue = 33
sage: x
33
sage: R(2)
33

It's easy to make this field no longer be public, but lots of things are using the fact that it is, so one needs to go through and make everything work correctly again.

Attachments (1)

trac-3571.patch (3.0 KB) - added by craigcitro 14 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 14 years ago by craigcitro

  • Status changed from new to assigned
  • Summary changed from ivalue field in integer_mod.pyx shouldn't be public to [with patch, needs review] ivalue field in integer_mod.pyx shouldn't be public

The attached patch fixes this issue, and in fact, gives about a 1.5-2X speedup on multiplying IntegerMod_ints. The interesting part is that previous to this patch, the ivalue field was occasionally being accessed as a Python attribute instead of a Cython attribute! That's why it broke if we made the field not public in the first place. Oops.

BEFORE:

sage: R = Integers(100) ; x = R(3) ; y = R(5)
sage: timeit('x*y')
625 loops, best of 3: 403 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 370 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 410 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 405 ns per loop

AFTER:

sage: R = Integers(100) ; x = R(3) ; y = R(5)
sage: timeit('x*y')
625 loops, best of 3: 190 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 213 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 174 ns per loop
sage: timeit('x*y')
625 loops, best of 3: 191 ns per loop

Changed 14 years ago by craigcitro

comment:2 Changed 14 years ago by robertwb

  • Summary changed from [with patch, needs review] ivalue field in integer_mod.pyx shouldn't be public to [with patch, positive review] ivalue field in integer_mod.pyx shouldn't be public

comment:3 Changed 14 years ago by mabshoff

  • Milestone changed from sage-3.4.1 to sage-3.3
  • Resolution set to fixed
  • Status changed from assigned to closed

Merged in Sage 3.3.alpha1

Cheers,

Michael

Note: See TracTickets for help on using tickets.