Opened 12 years ago

Closed 11 years ago

#10272 closed defect (fixed)

laurent series truncate behaviour different from power series truncate

Reported by: Timo Kluck Owned by: Martin Albrecht
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:

Status badges

Description (last modified by Timo Kluck)

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 Timo Kluck 11 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 12 years ago by Timo Kluck

Description: modified (diff)

comment:2 Changed 12 years ago by mhampton

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

comment:3 Changed 12 years ago by Timo Kluck

Status: newneeds_review

comment:4 Changed 12 years ago by Timo Kluck

Thanks, I just did.

comment:5 Changed 12 years ago by mhampton

Status: needs_reviewpositive_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 12 years ago by mhampton

Reviewers: Marshall Hampton

comment:7 Changed 12 years ago by Jeroen Demeyer

Milestone: sage-4.6.2

comment:8 Changed 12 years ago by Jeroen Demeyer

Status: positive_reviewneeds_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 12 years ago by Timo Kluck

Status: needs_workneeds_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 12 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
Work issues: 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 12 years ago by Timo Kluck

Status: needs_workneeds_review

Thanks, just added docstrings and examples.

comment:12 Changed 12 years ago by Jeroen Demeyer

Reviewers: Marshall HamptonMarshall Hampton, Jeroen Demeyer
Status: needs_reviewpositive_review

comment:13 Changed 12 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work
Work issues: docstringrebase

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 11 years ago by Timo Kluck

comment:14 Changed 11 years ago by Timo Kluck

Status: needs_workneeds_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 11 years ago by Karl-Dieter Crisman

Reviewers: Marshall Hampton, Jeroen DemeyerMarshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman
Status: needs_reviewpositive_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 11 years ago by Karl-Dieter Crisman (previous) (diff)

comment:16 Changed 11 years ago by Karl-Dieter Crisman

Keywords: sd40.5 added

comment:17 Changed 11 years ago by Jeroen Demeyer

Work issues: rebase

comment:18 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.1.beta3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.