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: | sage-8.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) | 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.)