Opened 14 years ago

Closed 14 years ago

#5152 closed defect (fixed)

[with patch; positive review] order of abelian group element is a rational number, but should be an integer

Reported by: was Owned by: somebody
Priority: minor Milestone: sage-3.3
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The line commented with "error here???" below is frightening:

File: /sage/groups/abelian_gps/abelian_group_element.py
Source Code (starting at line 268):
    def order(self):
        """
        Returns the (finite) order of this element or Infinity if this element
        does not have finite order.
 
        EXAMPLES:
            sage: F = AbelianGroup(3,[7,8,9]); F
            Multiplicative Abelian Group isomorphic to C7 x C8 x C9
            sage: F.gens()[2].order()
            9
            sage: a,b,c = F.gens()
            sage: (b*c).order()
            72
        """
        M = self.parent()
        if self == M(1):
            return Integer(1)
        invs = M.invariants()
        if self in M.gens():
            o = invs[list(M.gens()).index(self)]
            if o == 0:
                return infinity
            return o
        L = list(self.list())
        N = LCM([invs[i]/GCD(invs[i],L[i]) for i in range(len(invs)) if L[i]!=0])   ####### error here????
        if N == 0:
            return infinity
        else:
            return N

But what bugs me about it is:

sage: G = AbelianGroup(3,[7,8,9])
sage: type((G.0 * G.1).order())
<type 'sage.rings.rational.Rational'>

a simple coercion to Integer at the end of the function would fix this, or using instead of /. And add a doctest that has a type check so this doesn't get re-introduced.

Attachments (1)

trac-5152-abelian-gp-order.patch (1.6 KB) - added by wdj 14 years ago.
to be applied to 3.3.alpha3

Download all attachments as: .zip

Change History (5)

Changed 14 years ago by wdj

to be applied to 3.3.alpha3

comment:1 Changed 14 years ago by wdj

Please *ignore* trac-5152-abelian-gp-order.2.patch (2.8 kB). I don't know how that got there; hg_sage.export was not working the way I expected.

comment:2 Changed 14 years ago by wdj

  • Summary changed from order of abelian group element is a rational number, but should be an integer to [with patch, needs review] order of abelian group element is a rational number, but should be an integer

The patch trac-5152-abelian-gp-order.patch (1.6 kB) does exactly as instructed.

comment:3 Changed 14 years ago by was

  • Summary changed from [with patch, needs review] order of abelian group element is a rational number, but should be an integer to [with patch; positive review] order of abelian group element is a rational number, but should be an integer

comment:4 Changed 14 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.3.alpha4.

Cheers,

Michael

Note: See TracTickets for help on using tickets.