Opened 12 years ago
Closed 11 years ago
#1834 closed defect (fixed)
[with revised patch, with positive review] General linear group over ZZ hangs in __call__
Reported by: | kohel | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-3.1.2 |
Component: | linear algebra | Keywords: | |
Cc: | AlexGhitza | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
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 (1)
Change History (11)
comment:1 Changed 12 years ago by
- Milestone set to sage-2.10.1
comment:2 Changed 11 years ago by
- Cc AlexGhitza added
comment:3 Changed 11 years ago by
- Summary changed from General linear group over ZZ hangs in __call__ to [with patch, needs review] General linear group over ZZ hangs in __call__
comment:4 Changed 11 years ago by
- Owner changed from was to AlexGhitza
comment:5 Changed 11 years ago by
- Status changed from new to assigned
comment:6 Changed 11 years ago by
- 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
Changed 11 years ago by
comment:7 Changed 11 years ago by
Good idea. I have made the changes and replaced the patch with a new one.
comment:8 Changed 11 years ago by
- 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__
comment:9 Changed 11 years ago by
- 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!
comment:10 Changed 11 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.1.2.alpha4
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:
after:
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.