Opened 13 years ago
Closed 10 years ago
#1849 closed defect (duplicate)
[superseded by #6449] rewrite abelian groups
Reported by: | was | Owned by: | joyner |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | group theory | Keywords: | editor_roed |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
On Jan 19, 2008 8:00 AM, John Cremona <john.cremona@gmail.com> wrote: > > Either this is a bug or I have been teaching students the wrong thing for years: > > (in 2.9.3) > > sage: A=AbelianGroup([7,8]) > > sage: A.invariants() > [7,8] > > sage: A.elementary_divisors() > [7,8] > > There are two sets of invariants for a finite abelian group. The > "elementary divisors" should be a list of integers n_1,n_2,...,n_k > with each dividing the next such that A is the direct sum of cyclic > groups of order n_i. In this case A is cyclic so the list should be > just [56]. The other set is a list of prime powers and in this case > is indeed [7,8] though we could argue about the ordering. > > Judging by the docstrings, the implemeter of these things in Sage does > not agree with me, since the code seems to assume that the > "invariants" are what I am calling the elem. divisors, and the elem. > divisors are the prime-power factors of the invariants, sorted. In > which case the invariants is listed above are wrong! > > The Sage version of this is all explained in the documentation in file > sage/groups/abelian_gps/abelian_groups.py, attributed to David Joyner > . Please will all the algebraists out there tell me whether tey agree > with me or him! I'm not in a position to consult textbooks right now. > The docs there refer to Henri Cohen's books so it is possible that > there is a French/English split here BUT in any case the group I > defined about is *cyclic* so one of the two outputs is wrong, even if > we disagree which one! The problem is that when David Joyner implemented this, doing sage: A=AbelianGroup([7,8]) would immediately change the invariants to be the elementary divisors. I.e., it was *impossible* to make an abelian group with specified generator orders -- the would also change to be elementary divisors. This was so hugely painful to use that I fixed AbelianGroup so that one could create a group with arbitrary generator orders. Unfortunately, I evidently didn't correctly update the elementary_divisors function. William
Attachments (2)
Change History (11)
comment:1 Changed 13 years ago by
- Summary changed from bug in elementary_divisors for abelian groups to [with patch (part 1 of 2); not ready for review] rewrite abelian groups
Changed 13 years ago by
comment:2 Changed 13 years ago by
I just found this terrifying line in abelian_group.py
:
Hgensf = [x for x in Hgens if len(set(Ggens0).intersection(set(list(str(x)))))==0]
OUCH! This will completely break if one makes, e.g., abelian groups with generators that have more than one letter. For example, in current Sage
sage: G.<a,aa,aaa> = AbelianGroup(3,[5,0,0]) sage: G.subgroup([a]) Multiplicative Abelian Group isomorphic to C5, which is the subgroup of Multiplicative Abelian Group isomorphic to C5 x Z x Z generated by [a] sage: G.subgroup([aa]) --------------------------------------------------------------------------- <type 'exceptions.RuntimeError'> Traceback (most recent call last) /Users/was/<ipython console> in <module>() /Users/was/sage2/local/lib/python2.5/site-packages/sage/groups/abelian_gps/abelian_group.py in subgroup(self, gensH, names) 701 if not(gensH[i] in G): 702 raise TypeError, "Subgroup generators must belong to the given group." --> 703 return AbelianGroup_subgroup(self, gensH, names) 704 705 def __cmp__(self, right): /Users/was/sage2/local/lib/python2.5/site-packages/sage/groups/abelian_gps/abelian_group.py in __init__(self, ambient, gens, names) 927 gap.eval(cmd) 928 s2 = "gensH:=%s"%Hgensf #### remove from this the ones <--> 0 invar --> 929 gap.eval(s2) 930 s3 = 'H:=Subgroup(G,gensH)' 931 gap.eval(s3) /Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/gap.py in eval(self, x, newlines, strip) 307 if len(x) == 0 or x[len(x) - 1] != ';': 308 x += ';' --> 309 s = Expect.eval(self, x) 310 if newlines: 311 return s /Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/expect.py in eval(self, code, strip, **kwds) 705 try: 706 with gc_disabled(): --> 707 return '\n'.join([self._eval_line(L, **kwds) for L in code.split('\n') if L != '']) 708 except KeyboardInterrupt: 709 # DO NOT CATCH KeyboardInterrupt, as it is being caught /Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/gap.py in _eval_line(self, line, allow_use_file, wait_for_prompt) 508 return '' 509 else: --> 510 raise RuntimeError, message 511 512 except KeyboardInterrupt: <type 'exceptions.RuntimeError'>: Gap produced error output Variable: 'aa' must have a value executing gensH:=[aa];
comment:3 Changed 13 years ago by
comment:4 Changed 13 years ago by
comment:5 Changed 13 years ago by
- Keywords editor_roed added
comment:6 Changed 11 years ago by
- Report Upstream set to N/A
- Status changed from needs_work to needs_review
- Summary changed from [with patch (part 1 of 2); not ready for review] rewrite abelian groups to [superseded by #6449] rewrite abelian groups
I propose that we resolve this as "fixed" now #6449 has gone in.
comment:7 Changed 11 years ago by
- Milestone changed from sage-4.6 to sage-duplicate/invalid/wontfix
comment:8 Changed 11 years ago by
- Status changed from needs_review to positive_review
I'm deducing that you agree with my proposal to close this, so I'm setting it to "positive review". Release manager: please close as duplicate.
comment:9 Changed 10 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
this is part 1 -- doctests outside abelian groups directory will be broken.