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

Multiple bugs in Polynomial.reverse(degree) #21194

Closed
mezzarobba opened this issue Aug 8, 2016 · 14 comments
Closed

Multiple bugs in Polynomial.reverse(degree) #21194

mezzarobba opened this issue Aug 8, 2016 · 14 comments

Comments

@mezzarobba
Copy link
Member

Polynomial.reverse(d) over ℚ is inconsistent with the generic implementation. The name of the optional argument is different, and its interpretation is slightly different:

sage: x = polygen(QQ)
sage: y = polygen(QQbar)
sage: (x+1).reverse(1)
1
sage: (y+1).reverse(1)
x + 1

In addition, the documentation of the generic reverse() (which arguably should specify what reverse() is supposed to do for sage polynomials) incorrectly states that “the reverse polynomial will have the specified degree”:

sage: (y^2).reverse(5)
x^3

Finally, the generic implementation is buggy when the optional argument is zero.

Component: commutative algebra

Author: Marc Mezzarobba

Branch/Commit: 554500b

Reviewer: Vincent Delecroix

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

@mezzarobba

This comment has been minimized.

@mezzarobba

This comment has been minimized.

@mezzarobba
Copy link
Member Author

Commit: 160f6b3

@mezzarobba

This comment has been minimized.

@mezzarobba
Copy link
Member Author

Author: Marc Mezzarobba

@mezzarobba
Copy link
Member Author

Branch: u/mmezzarobba/21194-reverse

@mezzarobba mezzarobba changed the title Polynomial.reverse() over ℚ inconsistent with the generic implementation Multiple bugs in Polynomial.reverse(degree) Aug 8, 2016
@mezzarobba mezzarobba added this to the sage-7.4 milestone Aug 8, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2016

Changed commit from 160f6b3 to 08cee24

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2016

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

08cee24#21194 Fix multiple bugs in Polynomial.reverse()

@videlec
Copy link
Contributor

videlec commented Aug 9, 2016

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Aug 9, 2016

comment:5
sage: x = polygen(QQ)
sage: (x+1).reverse(2**64-1)
0

Your casting should be <unsigned long> (degree + 1).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2016

Changed commit from 08cee24 to 554500b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

554500b#21194 Fix handling of incorrect input

@mezzarobba
Copy link
Member Author

comment:9

Thanks!

@vbraun
Copy link
Member

vbraun commented Aug 13, 2016

Changed branch from u/mmezzarobba/21194-reverse to 554500b

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

3 participants