Opened 10 years ago
Closed 8 years ago
#12552 closed defect (fixed)
degree of polynomial_element is of type int
Reported by:  saraedum  Owned by:  malb 

Priority:  trivial  Milestone:  sage6.2 
Component:  commutative algebra  Keywords:  degree, polynomial 
Cc:  Merged in:  
Authors:  Julian Rueth  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  c45a290 (Commits, GitHub, GitLab)  Commit:  c45a290de9938da6fe5378e347b247a074fa6a42 
Dependencies:  #14482  Stopgaps: 
Description
The degree of a polynomial over RDF
is of type int
and not a sage Integer.
sage: R.<x> = RDF[] sage: type(x.degree()) <type 'int'> sage: x.degree().divides(6) Error
This is not consistent with polynomials over other rings:
sage: R.<x> = QQ[] sage: type(x.degree()) <type 'sage.rings.integer.Integer'> sage: x.degree().divides(6) True
The attached patch fixes this.
Attachments (1)
Change History (14)
Changed 10 years ago by
comment:1 Changed 10 years ago by
 Status changed from new to needs_review
comment:2 Changed 10 years ago by
 Status changed from needs_review to needs_work
comment:3 Changed 9 years ago by
 Branch set to u/saraedum/ticket/12552
 Dependencies set to #14482
comment:4 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:5 Changed 8 years ago by
Please make it clear whether the patch or the git branch should be merged. In the latter case, change the milestone to sage6.0.
comment:6 followup: ↓ 9 Changed 8 years ago by
This is a potential performance regression, How about using smallInteger() https://github.com/sagemath/sage/blob/master/src/sage/rings/integer.pxd#L33
comment:7 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:8 Changed 8 years ago by
 Commit set to c45a290de9938da6fe5378e347b247a074fa6a42
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c45a290  Change the degree of some polynomials to be an Integer.

comment:9 in reply to: ↑ 6 Changed 8 years ago by
 Status changed from needs_work to needs_review
Replying to robertwb:
This is a potential performance regression, How about using smallInteger() https://github.com/sagemath/sage/blob/master/src/sage/rings/integer.pxd#L33
I updated the code to use smallInteger()
. Is this what you had in mind? (I did not run doctests yet.)
comment:10 Changed 8 years ago by
Long tests pass in rings/. degree()
takes 1.4x longer time, IMHO not relevant. Premature optimization is the root of all evil  Don Knuth. If there are really badly affected cases write something like fast_degree()
.
comment:11 Changed 8 years ago by
 Status changed from needs_review to positive_review
comment:12 Changed 8 years ago by
 Reviewers set to Ralf Stephan
comment:13 Changed 8 years ago by
 Branch changed from u/saraedum/ticket/12552 to c45a290de9938da6fe5378e347b247a074fa6a42
 Resolution set to fixed
 Status changed from positive_review to closed
IIRC, this was a design decision for speed (though it could be revisited). We should at least be consistent between all the polynomial types.
As an aside, note that Integer(x) is preferable to ZZ(x).