#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:

Status badges

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 16 months ago by gh-DaveWitteMorris

Diagnosis: Laurent polynomials are stored as a pair consisting of a polynomial and an offset n that represents multiplication by x^n. Pynac's add::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.

comment:2 Changed 16 months ago by gh-DaveWitteMorris

  • Branch set to public/31645

comment:3 Changed 16 months ago by gh-DaveWitteMorris

  • 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:

b6f9170increment package version
96b0f8badd 32-bit known bug tags
ed6cac2Merge branch 'public/31554' of git://trac.sagemath.org/sage into t/31479/public/31479v2
2364860build/pkgs/pynac/package-version.txt: Bump patch level
b0b074eMerge #31479
eeb6cc2build/pkgs/pynac/package-version.txt: Bump patch level
04d0090Merge branch 'public/31479v2' of git://trac.sagemath.org/sage into t/31554/public/31554
74bcb8atrac 31645 handling of constant term in series
81e530badd doctest
a135bbbincrement pynac patch level

comment:4 Changed 16 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:5 Changed 16 months ago by gh-DaveWitteMorris

Thanks!

comment:6 Changed 16 months ago by mkoeppe

  • Priority changed from critical to blocker

comment:7 Changed 16 months ago by gh-DaveWitteMorris

  • Branch changed from public/31645 to public/31645r1

comment:8 Changed 16 months ago by gh-DaveWitteMorris

  • 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:

3fde2actrac 31479: pynac's poly_mul_expand
5501577add doctests
6bf6ec0increment package version
296d4c7add 32-bit known bug tags
ef84adfMerge branch 'public/31554' of git://trac.sagemath.org/sage into #31679polymulexpand
2b88eb5fix doctest for 32-bit
9aec019trac 31645 handling of constant term in series
89d65ffadd doctest
1da52deincrement pynac patch level again

comment:9 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:10 Changed 16 months ago by vbraun

  • Branch changed from public/31645r1 to 1da52de50e4f01c0e4c61c447217ff77c392b352
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.