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

major easy-to-fix but STUPID bug in gcd #4988

Closed
williamstein opened this issue Jan 16, 2009 · 4 comments
Closed

major easy-to-fix but STUPID bug in gcd #4988

williamstein opened this issue Jan 16, 2009 · 4 comments

Comments

@williamstein
Copy link
Contributor

This is stupid:

sage: gcd(3,6,2)
3

The problem is that there is an undocumented mysterious and not even used integer third input!

File:        /Users/was/s/local/lib/python2.5/site-packages/sage/rings/arith.py
Type:        <type 'function'>
Definition:  gcd(a, b, integer, **kwargs)
Docstring: 

    The greatest common divisor of a and b, or if a is a list and b is
    omitted the greatest common divisor of all elements of a.

    INPUT:
        a,b -- two elements of a ring with gcd
    or
        a -- a list or tuple of elements of a ring with gcd

    Additional keyword arguments are passed to the respectively called
    methods.

    EXAMPLES:
        sage: GCD(97,100)
        1
        sage: GCD(97*10^15, 19^20*97^2)
        97
        sage: GCD(2/3, 4/3)
        2/3
        sage: GCD([2,4,6,8])
        2
        sage: GCD(srange(0,10000,10))  # fast  !!
        10

This caused me a ton of confusion just now.

Component: number theory

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

@williamstein williamstein added this to the sage-3.3 milestone Jan 16, 2009
@williamstein williamstein self-assigned this Jan 16, 2009
@JohnCremona
Copy link
Member

Attachment: trac_4988.patch.gz

@JohnCremona
Copy link
Member

comment:1

Patch attached. I deleted the now redundant integer parameter (which once was used to tell the function to use integer-specific code and is now redundant). I added a relevant doctest and some more so hopefully William's confusion will never again occur (in someone else, I mean ;)). I discovered some places which still had "integer=True" in gcd calls and fixed those. I tested all rings/ and used search_src() to find any other places where "integer=True" might have been used, and found that search_src("integer=True") runs for ever while producing no output.

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Jan 21, 2009

comment:2

Fine by me.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 23, 2009

comment:3

Merged in Sage 3.3.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Jan 23, 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