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

univariate polynomial _xgcd only over fields #13438

Closed
saraedum opened this issue Sep 7, 2012 · 17 comments
Closed

univariate polynomial _xgcd only over fields #13438

saraedum opened this issue Sep 7, 2012 · 17 comments

Comments

@saraedum
Copy link
Member

saraedum commented Sep 7, 2012

sage.rings.polynomial.polynomial_element.Polynomial provides an implementation for _xgcd. This implementation is not correct for polynomials over arbitrary rings. Therefore it should be moved to sage.rings.polynomial.polynomial_element_generic.Polynomial_generic_field.

The way it currently is, doesn't cause any bugs (except for one which already has a stopgap warning) because only elements of a PID call the _xgcd method.


Apply: attachment: trac_13438_header.patch​

Component: basic arithmetic

Keywords: gcd, xgcd, beginner sd51

Author: Julian Rueth

Reviewer: Travis Scrimshaw, Michiel Kosters

Merged: sage-5.12.beta2

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

@saraedum
Copy link
Member Author

saraedum commented Sep 7, 2012

Changed keywords from xgcd to gcd, xgcd

@saraedum

This comment has been minimized.

@saraedum
Copy link
Member Author

saraedum commented Sep 7, 2012

Author: Julian Rueth

@saraedum
Copy link
Member Author

saraedum commented Sep 7, 2012

Dependencies: #13439

@saraedum
Copy link
Member Author

saraedum commented Sep 7, 2012

Changed dependencies from #13439 to none

@saraedum
Copy link
Member Author

Attachment: trac_13438.patch.gz

@saraedum

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Nov 25, 2012

comment:8

Two minor things I'd like to see changed:

  • The INPUT: bullet is indented one to many times
  • Change the OUTPUT: to
Polynomials ``g``, ``u``, and ``v`` such that ``g = u * self + v * other``. 

Thanks.

@sagetrac-mkosters
Copy link
Mannequin

sagetrac-mkosters mannequin commented Jul 24, 2013

comment:9

Attachment: trac_13438_beta.patch.gz

Beta patch should replace original patch. Original patch fails due to intermediate changes in sage (as of 5.10). Beta patch also includes tscrim's suggested doc changes.

@tscrim
Copy link
Collaborator

tscrim commented Jul 24, 2013

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 24, 2013

comment:10

The patch is missing the header information. You'll need to run

sage -hg qrefresh -e
sage -hg export trac_13438_beta.patch > path/to/sage/devel/sage/.hg/patches/trac_13438_beta.patch

(or save it to wherever you'd like to upload the patch from). You should also add yourself as a reviewer of the patch with your real name.

Thanks,

Travis

@sagetrac-mkosters
Copy link
Mannequin

sagetrac-mkosters mannequin commented Jul 25, 2013

comment:11

Attachment: trac_13438_header.patch.gz

The new patch (trac_13438_header.patch​) includes header and commit.

@sagetrac-mkosters
Copy link
Mannequin

sagetrac-mkosters mannequin commented Jul 25, 2013

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Michiel Kosters

@tscrim
Copy link
Collaborator

tscrim commented Jul 25, 2013

comment:12

Looks good to me. Thank you.

For patchbot:

Apply: trac_13438_header.patch​

@tscrim

This comment has been minimized.

@mstreng
Copy link
Contributor

mstreng commented Jul 25, 2013

Changed keywords from gcd, xgcd to gcd, xgcd, beginner sd51

@jdemeyer jdemeyer removed this from the sage-5.11 milestone Jul 25, 2013
@jdemeyer jdemeyer added this to the sage-5.12 milestone Jul 25, 2013
@jdemeyer
Copy link
Contributor

Merged: sage-5.12.beta2

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

5 participants