Opened 8 years ago

Closed 7 years ago

#10272 closed defect (fixed)

laurent series truncate behaviour different from power series truncate

Reported by: tkluck Owned by: malb
Priority: minor Milestone: sage-5.1
Component: commutative algebra Keywords: laurent series, truncate sd40.5
Cc: Merged in: sage-5.1.beta3
Authors: Timo Kluck Reviewers: Marshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by tkluck)

There is the following difference in behaviour between truncate() for power series and laurent series:

sage: P.<p> = PowerSeriesRing(QQ)
sage: L.<l> = LaurentSeriesRing(QQ)
sage: f = p^2 + p^3 + p^5 +p^6 + O(p^7)
sage: g = l^2 + l^3 + l^5 +l^6 + O(l^7)
sage: f.truncate(6)
p^5 + p^3 + p^2
sage: g.truncate(6)
l^2 + l^3 + l^5 + O(l^6)
sage: f.truncate_powerseries(6)
p^2 + p^3 + p^5 + O(p^6)

The problem is simply that g.truncate() calls truncate_powerseries() on its underlying power series.

The attached patch changes LaurentSeries?.truncate() such that is calls truncate() on the underlying powerseries. For the current behaviour, it also adds a method truncate_laurentseries() which calls truncate_powerseries() on the underlying powerseries.

In my opinion, the methods truncate_powerseries and truncate_laurentseries are superfluous since they are equivalent to add_bigoh().

Attachments (1)

trac_10272_laurent_series_truncate.patch (2.0 KB) - added by tkluck 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by tkluck

  • Description modified (diff)

comment:2 Changed 8 years ago by mhampton

Is this ready for review? If so, you should change its status to "needs review".

comment:3 Changed 8 years ago by tkluck

  • Status changed from new to needs_review

comment:4 Changed 8 years ago by tkluck

Thanks, I just did.

comment:5 Changed 8 years ago by mhampton

  • Status changed from needs_review to positive_review

Looks good, positive review. Doesn't break any doctests in rings because Laurent series are so poorly tested! Only bad thing is that you should include the ticket number on the patch, i.e. this one should have been named something like trac_10272_laurent_series_truncate.patch. This helps the release manager and others in a variety of ways.

comment:6 Changed 8 years ago by mhampton

  • Reviewers set to Marshall Hampton

comment:7 Changed 8 years ago by jdemeyer

  • Milestone set to sage-4.6.2

comment:8 Changed 8 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Patches should be created using hg export tip, not hg diff. I also remind you that a patch should have a sensible commit message (use hg qrefresh -e for that) and that the ticket number should appear on the first line of the commit message.

comment:9 Changed 8 years ago by tkluck

  • Status changed from needs_work to needs_review

I just added the patch in the right format, i.e. the output of hg_sage.export() instead of a normal diff.

comment:10 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues set to docstring

Please read http://sagemath.org/doc/developer/conventions.html#documentation-strings and fix the documentation of this new function accordingly. In particular, you should add at least one example.

comment:11 Changed 8 years ago by tkluck

  • Status changed from needs_work to needs_review

Thanks, just added docstrings and examples.

comment:12 Changed 8 years ago by jdemeyer

  • Reviewers changed from Marshall Hampton to Marshall Hampton, Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:13 Changed 8 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Work issues changed from docstring to rebase

This needs to be rebased:

applying /scratch/jdemeyer/merger/downloads/trac_10272_laurent_series_truncate.patch
patching file sage/rings/laurent_series_ring_element.pyx
Hunk #1 FAILED at 725
1 out of 1 hunks FAILED -- saving rejects to file sage/rings/laurent_series_ring_element.pyx.rej
abort: patch failed to apply

Changed 7 years ago by tkluck

comment:14 Changed 7 years ago by tkluck

  • Status changed from needs_work to needs_review

Sorry for not having done this earlier, I totally forgot. I just uploaded a new version that should apply cleanly against the current version. Please review.

comment:15 Changed 7 years ago by kcrisman

  • Reviewers changed from Marshall Hampton, Jeroen Demeyer to Marshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman
  • Status changed from needs_review to positive_review
sage -t  "devel/sage-main/sage/rings/laurent_series_ring_element.pyx"
	 [2.1 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 2.1 seconds

And it still applies.

Last edited 7 years ago by kcrisman (previous) (diff)

comment:16 Changed 7 years ago by kcrisman

  • Keywords sd40.5 added

comment:17 Changed 7 years ago by jdemeyer

  • Work issues rebase deleted

comment:18 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.