Opened 14 months ago
Closed 13 months ago
#31645 closed defect (fixed)
incorrect handling of constant term when creating power series
Reported by:  ghDaveWitteMorris  Owned by:  

Priority:  blocker  Milestone:  sage9.3 
Component:  symbolics  Keywords:  pynac, series 
Cc:  Merged in:  
Authors:  Dave Morris  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  1da52de (Commits, GitHub, GitLab)  Commit:  1da52de50e4f01c0e4c61c447217ff77c392b352 
Dependencies:  #31679  Stopgaps: 
Description
As reported by user TBK in this sage devel thread, the series
method sometimes returns an incorrect power series in which a constant term has erroneously been multiplied by a power of x:
sage: ((1sqrt(1x))/x + 0).series(x,3) 1/2 + 1/8*x + 1/16*x^2 + Order(x^3) # correct sage: ((1sqrt(1x))/x + 123).series(x,3) 123*x^(1) + 1/2 + 1/8*x + 1/16*x^2 + Order(x^3) # wrong !!!
Change History (10)
comment:1 Changed 14 months ago by
comment:2 Changed 14 months ago by
 Branch set to public/31645
comment:3 Changed 14 months ago by
 Commit set to a135bbb6364d1feb3a806bbdb2ca0f4334ae1756
 Dependencies set to #31554
 Status changed from new to needs_review
The PR fixes the bug by moving the handling of the constant term to the initialization phase, where the offset is known to be 0.
With this PR applied, we get the correct result for the example in the ticket description:
sage: ((1sqrt(1x))/x + 123).series(x,3) 247/2 + 1/8*x + 1/16*x^2 + Order(x^3)
As a doctest, the PR uses the following very simple example that was also giving a wrong answer:
sage: (x^(1) + 1).series(x,1) # wrong answer !!! 2*x^(1) + Order(x)
My recent pynac patches had merge conflicts, but I rebased this one on #31554 (hence the dependency), so I think it should merge cleanly. Only the last 3 commits come from this ticket ("trac 31645 handling ...", "add doctest", and "increment pynac patch level").
This is a critical bug, so I hope it can be merged in 9.3.
Last 10 new commits:
b6f9170  increment package version

96b0f8b  add 32bit known bug tags

ed6cac2  Merge branch 'public/31554' of git://trac.sagemath.org/sage into t/31479/public/31479v2

2364860  build/pkgs/pynac/packageversion.txt: Bump patch level

b0b074e  Merge #31479

eeb6cc2  build/pkgs/pynac/packageversion.txt: Bump patch level

04d0090  Merge branch 'public/31479v2' of git://trac.sagemath.org/sage into t/31554/public/31554

74bcb8a  trac 31645 handling of constant term in series

81e530b  add doctest

a135bbb  increment pynac patch level

comment:4 Changed 14 months ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:5 Changed 14 months ago by
Thanks!
comment:6 Changed 13 months ago by
 Priority changed from critical to blocker
comment:7 Changed 13 months ago by
 Branch changed from public/31645 to public/31645r1
comment:8 Changed 13 months ago by
 Commit changed from a135bbb6364d1feb3a806bbdb2ca0f4334ae1756 to 1da52de50e4f01c0e4c61c447217ff77c392b352
 Dependencies changed from #31554 to #31679
 Status changed from positive_review to needs_review
Rebased on #31679 (which replaced #31554), and updated the patch version again.
New commits:
3fde2ac  trac 31479: pynac's poly_mul_expand

5501577  add doctests

6bf6ec0  increment package version

296d4c7  add 32bit known bug tags

ef84adf  Merge branch 'public/31554' of git://trac.sagemath.org/sage into #31679polymulexpand

2b88eb5  fix doctest for 32bit

9aec019  trac 31645 handling of constant term in series

89d65ff  add doctest

1da52de  increment pynac patch level again

comment:9 Changed 13 months ago by
 Status changed from needs_review to positive_review
comment:10 Changed 13 months ago by
 Branch changed from public/31645r1 to 1da52de50e4f01c0e4c61c447217ff77c392b352
 Resolution set to fixed
 Status changed from positive_review to closed
Diagnosis: Laurent polynomials are stored as a pair consisting of a polynomial and an offset
n
that represents multiplication byx^n
. Pynac'sadd::useries
method fails to account for the offset when adding the constant term to a power series.This should be easy to fix, so I should be able to upload a PR soon.