Opened 15 years ago
Closed 14 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: | N/A | 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 15 years ago by
Milestone: | → sage-2.10.1 |
---|
comment:2 Changed 14 years ago by
Cc: | AlexGhitza added |
---|
comment:3 Changed 14 years ago by
Summary: | General linear group over ZZ hangs in __call__ → [with patch, needs review] General linear group over ZZ hangs in __call__ |
---|
comment:4 Changed 14 years ago by
Owner: | changed from was to AlexGhitza |
---|
comment:5 Changed 14 years ago by
Status: | new → assigned |
---|
comment:6 Changed 14 years ago by
Component: | algebraic geometry → linear algebra |
---|---|
Summary: | [with patch, needs review] General linear group over ZZ hangs in __call__ → [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 14 years ago by
Attachment: | 1834-gl_z_call.patch added |
---|
comment:7 Changed 14 years ago by
Good idea. I have made the changes and replaced the patch with a new one.
comment:8 Changed 14 years ago by
Summary: | [with patch, with review, work needed] General linear group over ZZ hangs in __call__ → [with revised patch, needs review] General linear group over ZZ hangs in __call__ |
---|
comment:9 Changed 14 years ago by
Summary: | [with revised patch, needs review] General linear group over ZZ hangs in __call__ → [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 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → 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.