Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

order of abelian group element is a rational number, but should be an integer #5152

Closed
williamstein opened this issue Feb 1, 2009 · 4 comments

Comments

@williamstein
Copy link
Contributor

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.

Component: basic arithmetic

Issue created by migration from https://trac.sagemath.org/ticket/5152

@wdjoyner
Copy link

wdjoyner commented Feb 1, 2009

to be applied to 3.3.alpha3

@wdjoyner
Copy link

wdjoyner commented Feb 1, 2009

comment:1

Attachment: trac-5152-abelian-gp-order.patch.gz

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.

@wdjoyner
Copy link

wdjoyner commented Feb 1, 2009

comment:2

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

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Feb 2, 2009

comment:4

Merged in Sage 3.3.alpha4.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Feb 2, 2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants