#13501: Fix two bugs in sage.misc.c3's implementation of the algorithm C3
-------------------------------------------+--------------------------------
       Reporter:  nthiery                  |         Owner:  nthiery     
           Type:  defect                   |        Status:  needs_review
       Priority:  major                    |     Milestone:  sage-5.4    
      Component:  categories               |    Resolution:              
       Keywords:  method resolution order  |   Work issues:              
Report Upstream:  N/A                      |     Reviewers:  Simon King  
        Authors:  Nicolas M. ThiƩry        |     Merged in:              
   Dependencies:  #12895                   |      Stopgaps:              
-------------------------------------------+--------------------------------

Comment (by SimonKing):

 With a little cdef'ing, namely
 {{{#!diff
 diff --git a/sage/misc/c3.pyx b/sage/misc/c3.pyx
 --- a/sage/misc/c3.pyx
 +++ b/sage/misc/c3.pyx
 @@ -196,6 +196,8 @@
      nbheads = len(heads)
      cdef object O, X
      cdef bint next_item_found
 +    cdef set tail_set
 +    cdef list tail_list

      while nbheads:
          for i from 0 <= i < nbheads:
 @@ -205,7 +207,8 @@
              for j from 0 <= j < nbheads:
                  if j == i:
                      continue
 -                if O in tailsets[j]:
 +                tail_set = tailsets[j]
 +                if O in tail_set:
                      next_item_found = False
                      break
              if next_item_found:
 @@ -215,11 +218,13 @@
                  # j goes down so that ``del heads[j]`` does not screw up
 the numbering
                  for j from nbheads > j >= 0:
                      if heads[j] is O:
 -                        try:
 -                            X = tails[j].pop()
 +                        tail_list = tails[j]
 +                        if tail_list:
 +                            X = tail_list.pop()
                              heads[j] = X
 -                            tailsets[j].remove(X)
 -                        except IndexError:
 +                            tail_set = tailsets[j]
 +                            tail_set.remove(X)
 +                        else:
                              del heads[j]
                              del tails[j]
                              del tailsets[j]
 }}}
 I get
 {{{
    ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  7000/500    0.277    0.000    0.819    0.002 {sage.misc.c3.C3_algorithm}
 }}}

 The differences really are tiny. However, here is why I removed the `try:
 ... except IndexError:`:
 {{{
 sage: cython("""
 ....: def test1(list L):
 ....:     cdef list l
 ....:     for l in L:
 ....:         try:
 ....:             l.pop()
 ....:         except IndexError:
 ....:             l.append(0)
 ....: def test2(list L):
 ....:     cdef list l
 ....:     for l in L:
 ....:         if l:
 ....:             l.pop()
 ....:         else:
 ....:             l.append(0)
 ....: """)
 sage: L = [[] for _ in range(10000)]
 sage: %timeit test1(L)
 125 loops, best of 3: 7.07 ms per loop
 sage: L = [[] for _ in range(10000)]
 sage: %timeit test2(L)
 125 loops, best of 3: 1.94 ms per loop
 }}}

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13501#comment:16>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica, 
and MATLAB

-- 
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sage-trac?hl=en.

Reply via email to