Opened 8 years ago

Last modified 8 years ago

#12020 closed defect

bug in zero_matrix rewrite in matrix_space.py — at Version 4

Reported by: was Owned by: jason, was
Priority: blocker Milestone: sage-4.8
Component: linear algebra Keywords:
Cc: vbraun Merged in:
Authors: William Stein Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Observe this:

sage: D=DirichletGroup(20);
sage: g=D[7].extend(400); #order 4 character
sage: M=ModularSymbols(g,2,sign=1).cuspidal_subspace()
sage: N=M.new_subspace()
TypeError                                 Traceback (most recent call last)

/mnt/usb1/scratch/wstein/ref/sage-4.8.alpha2-sage.math.washington.edu-x86_64-Linux/local/lib/python2.6/site-packages/sage/rings/number_field/number_field.pyc in _coerce_non_number_field_element_in(self, x)
   5399         except (TypeError, AttributeError), msg:
   5400             pass
-> 5401         raise TypeError, type(x)
   5402 
   5403     def _coerce_from_str(self, x):
TypeError: <type 'NoneType'>

Debugging we see that the problem is that this code in matrix_space.py:

    479         if entries is None or entries == 0:
    480             if self._copy_zero: # faster to copy than to create a new one.
    481                 return self.zero_matrix().__copy__()
    482             else:
--> 483                 return self.__matrix_class(self, None, coerce=coerce, copy=copy)

That None in the last line is then coerced for some reason into a number field (Cyclotomic Field of order 4 and degree) which makes no sense now...

This bug was introduced in #11589.

Apply trac_12020.2.patch

Change History (6)

comment:1 Changed 8 years ago by was

I figured this out. This bug was introduced by #11589 (by Simon King and Martin Albrecht). Basically the zero_matrix() method's code was "mangled/changed" (?) when being copied into the init method by replacing "0" by "None". This breaks the code, since there is no reason that None be coercible into a ring R, and I don't think that has to be supported because, e.g., in Python int(None) is an error. Also, this is a weird change since the zero_matrix method in the new code still uses 0.

Patch coming up.

comment:2 Changed 8 years ago by was

Here is a simple test to reveal the bug. Small size matrices don't show the bug, probably because of some arbitrary caching parameter in #11589, which is probably why this bug slipped through our test framework (we *really* need a random larger testing framework!):

sage: A = MatrixSpace(CyclotomicField(4),60,30)(0)
sage: A.augment(A)
boom!

comment:3 Changed 8 years ago by was

  • Summary changed from major (BLOCKER!) bug probably in matrix_space.py to bug in zero_matrix rewrite in matrix_space.py

Changed 8 years ago by was

Changed 8 years ago by jdemeyer

Same patch with corrected doc formatting

comment:4 Changed 8 years ago by jdemeyer

  • Authors set to William Stein
  • Description modified (diff)
  • Status changed from new to needs_review
Note: See TracTickets for help on using tickets.