Opened 5 years ago

Closed 5 years ago

#21194 closed defect (fixed)

Multiple bugs in Polynomial.reverse(degree)

Reported by: mmezzarobba Owned by:
Priority: major Milestone: sage-7.4
Component: commutative algebra Keywords:
Cc: Merged in:
Authors: Marc Mezzarobba Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 554500b (Commits, GitHub, GitLab) Commit: 554500b7cecafee1c8596bc4a5c0300a5e82a89b
Dependencies: Stopgaps:

Status badges

Description (last modified by mmezzarobba)

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.

Change History (10)

comment:1 Changed 5 years ago by mmezzarobba

  • Description modified (diff)

comment:2 Changed 5 years ago by mmezzarobba

  • Description modified (diff)

comment:3 Changed 5 years ago by mmezzarobba

  • Authors set to Marc Mezzarobba
  • Branch set to u/mmezzarobba/21194-reverse
  • Commit set to 160f6b35e9253d82362c88aaf82b28cc7dfdc539
  • Description modified (diff)
  • Milestone set to sage-7.4
  • Status changed from new to needs_review
  • Summary changed from Polynomial.reverse() over ℚ inconsistent with the generic implementation to Multiple bugs in Polynomial.reverse(degree)
Last edited 5 years ago by mmezzarobba (previous) (diff)

comment:4 Changed 5 years ago by git

  • Commit changed from 160f6b35e9253d82362c88aaf82b28cc7dfdc539 to 08cee2465de3ac7c8289742705ad117a41d37430

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

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

comment:5 Changed 5 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to needs_work 
sage: x = polygen(QQ)
sage: (x+1).reverse(2**64-1)
0

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

comment:6 Changed 5 years ago by git

  • Commit changed from 08cee2465de3ac7c8289742705ad117a41d37430 to 554500b7cecafee1c8596bc4a5c0300a5e82a89b

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

​554500b#21194 Fix handling of incorrect input

comment:7 Changed 5 years ago by mmezzarobba

  • Status changed from needs_work to needs_review

comment:8 Changed 5 years ago by vdelecroix

  • Status changed from needs_review to positive_review

comment:9 Changed 5 years ago by mmezzarobba

Thanks!

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/mmezzarobba/21194-reverse to 554500b7cecafee1c8596bc4a5c0300a5e82a89b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.