Ticket #1834 (closed defect: fixed)

Opened 10 months ago

Last modified 3 months ago

[with revised patch, with positive review] General linear group over ZZ hangs in __call__

Reported by: kohel Assigned to: AlexGhitza
Priority: major Milestone: sage-3.1.2
Component: linear algebra Keywords:
Cc: AlexGhitza

Description

sage: G = GL(3,GF(101)) sage: G([[1,0,1],[0,1,0],[0,0,1]])

[1 0 1] [0 1 0] [0 0 1]

works fine, but

sage: G = GL(3,ZZ) sage: G([[1,0,1],[0,1,0],[0,0,1]])

This should not try to find a solution to the word problem.

Attachments

1834-gl_z_call.patch (4.9 kB) - added by AlexGhitza on 09/01/2008 04:08:21 PM.

Change History

01/23/2008 01:29:01 AM changed by mabshoff

  • milestone set to sage-2.10.1.

08/26/2008 04:06:58 AM changed by AlexGhitza

  • cc set to AlexGhitza.

08/29/2008 06:37:59 AM changed by AlexGhitza

  • summary changed from General linear group over ZZ hangs in __call__ to [with patch, needs review] General linear group over ZZ hangs in __call__.

This has been around for a while, and it's been bugging me. I fixed it by writing methods __call__ and __contains__ for the class GeneralLinearGroup_generic, so that the GAP ones (which hang over ZZ) don't get used. A pleasant side effect is that things are now faster for the cases that were working before (i.e. over finite fields):

before:

sage: G = GL(5, GF(next_prime(6*10^4)))                                    
sage: m = [[1,0,1,0,2],[0,1,0,1,1],[0,0,1,0,0],[0,0,0,1,1],[0,0,0,0,1]]                              
sage: timeit("G(m)")                                                                                     
25 loops, best of 3: 9.56 ms per loop

after:

sage: G = GL(5, GF(next_prime(6*10^4)))                                    
sage: m = [[1,0,1,0,2], [0,1,0,1,1], [0,0,1,0,0], [0,0,0,1,1], [0,0,0,0,1]]                              
sage: timeit("G(m)")                                                                                     
625 loops, best of 3: 459 µs per loop

The same issue comes up for all the matrix groups. For the moment, I have only dealt with the really easy cases of GL and SL. If this gets approved and merged, I will open a new ticket for the other classes of groups and deal with them in a similar way.

09/01/2008 02:15:04 AM changed by AlexGhitza

  • owner changed from was to AlexGhitza.

09/01/2008 03:03:12 AM changed by AlexGhitza

  • status changed from new to assigned.

09/01/2008 12:38:43 PM changed by cremona

  • component changed from algebraic geometry to linear algebra.
  • summary changed from [with patch, needs review] General linear group over ZZ hangs in __call__ to [with patch, with review, work needed] General linear group over ZZ hangs in __call__.

Basically ok, but I would make the following changes to both cases (GL and SL):

Use a try: except: block to catch when coercion in the matrix space fails, with the error message "Cannot coerce ... to a matrix". Then catch the non-invertible matrices in the GL case with the error message "... is not an invertible matrix", and similarly in the SL case with "... does not have determinant 1".

I think this alternative error handling will produce better output.

PS As this is not "algebraic geometry" I changed the Component to Linear Algebra

09/01/2008 04:08:21 PM changed by AlexGhitza

  • attachment 1834-gl_z_call.patch added.

09/01/2008 04:09:28 PM changed by AlexGhitza

Good idea. I have made the changes and replaced the patch with a new one.

09/01/2008 04:17:17 PM changed by AlexGhitza

  • summary changed from [with patch, with review, work needed] General linear group over ZZ hangs in __call__ to [with revised patch, needs review] General linear group over ZZ hangs in __call__.

09/02/2008 01:37:33 AM changed by cremona

  • summary changed from [with revised patch, needs review] General linear group over ZZ hangs in __call__ to [with revised patch, with positive review] General linear group over ZZ hangs in __call__.

Excellent. These are much more helpful messages.

The new patch applies ok to 3.1.2.alpha3 (there was a little fuzz:

applying /home/john/1834-gl_z_call.patch
patching file sage/groups/matrix_gps/matrix_group.py
Hunk #1 succeeded at 13 with fuzz 1 (offset 3 lines).

but nothing serious). All doctests in sage.groups.matrix_groups pass. Publish!

09/02/2008 04:02:34 AM changed by mabshoff

  • status changed from assigned to closed.
  • resolution set to fixed.

Merged in Sage 3.1.2.alpha4