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.

Sign up for GitHub

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

Jump to bottom

Improvements to AGM #7739

Closed
robertwb opened this issue Dec 18, 2009 · 9 comments
Closed

Improvements to AGM #7739

robertwb opened this issue Dec 18, 2009 · 9 comments
Assignees
@aghitza
Labels
c: basic arithmetic p: major / 3 t: enhancement
Milestone
sage-4.3.1

Comments

@robertwb
Copy link
Contributor

Native (much faster) agm for RDF and CDF, optimized and document agm for RR.

Inspired by, but somewhat orthogonal to, #7719.

CC: @JohnCremona

Component: basic arithmetic

Author: Robert Bradshaw

Reviewer: John Cremona

Merged: sage-4.3.1.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/7739
@robertwb robertwb added this to the sage-4.3.1 milestone Dec 18, 2009
@JohnCremona
Copy link
Member

comment:1

Look basically good. Robert, do you want to add the test for a=0 or b=0 or a=-b in the complex_double case, and also perhaps a=0 or b=0 for the real cases?

@robertwb
Copy link
Contributor Author

robertwb commented Jan 7, 2010

comment:2

Attachment: 7739-cdfrdf-agm.patch.gz

Good idea, I added some degenerate tests and refreshed the patch.

@JohnCremona
Copy link
Member

Attachment: 7739-cdfrdf-agm.2.patch.gz

corrects typo in previous patch (which it replaces)

@JohnCremona
Copy link
Member

comment:3

There's a typo (sgm for agm) in the docstring (line 1944 of complex_double). I edited the patch to fix that.

Otherwise I'm quite happy -- applies to 4.3 and tests in sage/rings/{real,complex}* all pass. So: positive review!

@JohnCremona
Copy link
Member

Reviewer: John Cremona

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 13, 2010

Merged: 4.3.1.alpha2

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 13, 2010

Author: Robert Bradshaw

@rlmill rlmill mannequin removed the s: positive review label Jan 13, 2010
@rlmill rlmill mannequin closed this as completed Jan 13, 2010
@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 13, 2010

Changed merged from 4.3.1.alpha2 to sage-4.3.1.alpha2

@robertwb
Copy link
Contributor Author

comment:6

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aghitza aghitza

Labels
c: basic arithmetic p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-4.3.1
Development

No branches or pull requests
3 participants
@robertwb @aghitza @JohnCremona