Opened 4 years ago
Closed 4 years ago
#22936 closed defect (fixed)
Newton polygon of polynomials over power series with infinite precision
Reported by:  bruno  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  algebra  Keywords:  newton polygon 
Cc:  caruso  Merged in:  
Authors:  Xavier Caruso  Reviewers:  Bruno Grenet, Julian Rüth 
Report Upstream:  N/A  Work issues:  
Branch:  f5fbc72 (Commits, GitHub, GitLab)  Commit:  f5fbc720f82b11787301b36efe8359c7e1dc653b 
Dependencies:  Stopgaps: 
Description
Computing the Newton polygon of a polynomial with coefficients in a ring of power series fails if the coefficients have infinite precision:
sage: S.<x> = PowerSeriesRing(GF(5)) sage: R.<y> = S[] sage: p = x^2+y+x*y^2 sage: p.newton_polygon() Traceback (most recent call last): ... IndexError: list index out of range
Change History (10)
comment:1 Changed 4 years ago by
 Branch set to u/caruso/newton_polygon_infinite_prec
comment:2 Changed 4 years ago by
 Commit set to 8204e97c49c9ef9b1dad0d3b20c3a2cceb0be199
 Status changed from new to needs_review
comment:3 Changed 4 years ago by
 Commit changed from 8204e97c49c9ef9b1dad0d3b20c3a2cceb0be199 to 61cb00ccf1648775300d83c7896ae058a51b0ebb
Branch pushed to git repo; I updated commit sha1. New commits:
61cb00c  Newton polygon and slope factorization fixed

comment:4 Changed 4 years ago by
 Commit changed from 61cb00ccf1648775300d83c7896ae058a51b0ebb to ac0d97598c09c76e7a7b8ccf56e4bad9d6a1ce5a
Branch pushed to git repo; I updated commit sha1. New commits:
ac0d975  Replace TESTS: by TESTS::

comment:5 Changed 4 years ago by
Looks good, except for the TESTS:
. Positive review if the patchbot is happy.
New commits:
ac0d975  Replace TESTS: by TESTS::

comment:6 Changed 4 years ago by
 Status changed from needs_review to needs_work
Two One comments:
 You changed too many
TESTS:
byTESTS::
: There should be double colons only whenTESTS:
is followed by an actual test, not when it is followed by a sentence that explains the test.
It would be nice to add a test for slope factorization.(The test already exists...)
comment:7 Changed 4 years ago by
 Commit changed from ac0d97598c09c76e7a7b8ccf56e4bad9d6a1ce5a to f5fbc720f82b11787301b36efe8359c7e1dc653b
comment:8 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 4 years ago by
 Reviewers set to Bruno Grenet, Julian Rüth
 Status changed from needs_review to positive_review
LGTM, thanks for the patch!
comment:10 Changed 4 years ago by
 Branch changed from u/caruso/newton_polygon_infinite_prec to f5fbc720f82b11787301b36efe8359c7e1dc653b
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
I attach a small patch fixing this issue. I've also updated the method
_factor_of_degree
in order to make the slope factorization work correctly. (Otherwise, at infinite precision, it may never stop.)