Opened 14 months ago
Closed 13 months ago
#31645 closed defect (fixed)
incorrect handling of constant term when creating power series
Reported by: | gh-DaveWitteMorris | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-9.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: ((1-sqrt(1-x))/x + 0).series(x,3) 1/2 + 1/8*x + 1/16*x^2 + Order(x^3) # correct sage: ((1-sqrt(1-x))/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: ((1-sqrt(1-x))/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 32-bit known bug tags
|
ed6cac2 | Merge branch 'public/31554' of git://trac.sagemath.org/sage into t/31479/public/31479v2
|
2364860 | build/pkgs/pynac/package-version.txt: Bump patch level
|
b0b074e | Merge #31479
|
eeb6cc2 | build/pkgs/pynac/package-version.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 32-bit known bug tags
|
ef84adf | Merge branch 'public/31554' of git://trac.sagemath.org/sage into #31679polymulexpand
|
2b88eb5 | fix doctest for 32-bit
|
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.