search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.34k stars 454 forks source link

Make big_oh notation work for any element that has a big_oh() method #10271

Open tkluck opened 13 years ago

tkluck commented 13 years ago

One can use the big-oh notation

O(xn)

when x is a generator of a power series or a laurent series. It is convenient to also be able to use this notation when the parent of x is another ring (for example, a subclass of a power series ring).

The current implementation in rings/big_oh.py imports a large number of types, which slows down the Sage startup. Elements should implement their own big_oh() methods, and the function O(x) should simply call x.big_oh().

Depends on #11726 Depends on #11719

Component: commutative algebra

Keywords: sd32

Work Issues: add doctest

Author: Tom Boothby

Reviewer: Timo Kluck

Issue created by migration from https://trac.sagemath.org/ticket/10271

tkluck commented 13 years ago

Attachment: bigoh.patch.gz

patch for sage.rings.big_oh

tkluck commented 13 years ago

Attachment: trac_10271_bigoh_extension.patch.gz

tkluck commented 13 years ago
comment:2

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

a9bc4e33-7b98-4180-affc-8dfcef89e22b commented 13 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-On can use the big-oh notation
+One can use the big-oh notation

 O(x^n)
a9bc4e33-7b98-4180-affc-8dfcef89e22b commented 13 years ago

Reviewer: Mariah Lenox

a9bc4e33-7b98-4180-affc-8dfcef89e22b commented 13 years ago
comment:4

Applied patch to sage-4.7.1.alpha2, did 'sage -b', then 'make testlong'. All tests passed. Positive review!

jdemeyer commented 13 years ago
comment:5

I think this patch needs an extra doctest to demonstrate what it does and that it works as expected.

jdemeyer commented 13 years ago

Work Issues: add doctest

williamstein commented 13 years ago
comment:6

The following behavior is bad. In both cases, I think one should get an error.

sage: R.<x> = QQ[[]]
sage: O(1+x^10)
O(x^10)
sage: f = x*(1/x + x^9); type(f)
<type 'sage.rings.laurent_series_ring_element.LaurentSeries'>
sage: O(f)
O(x^0)
boothby commented 13 years ago

Description changed:

--- 
+++ 
@@ -1,7 +1,7 @@
 One can use the big-oh notation

-O(x^n)
+O(x<sup>n</sup>)

 when x is a generator of a power series or a laurent series. It is convenient to also be able to use this notation when the parent of x is another ring (for example, a subclass of a power series ring).

-For this, we only need x to implement x.degree() and x.add_bigoh(). I made a patch for sage.rings.big_oh that tests for this and returns x.add_bigoh(x.degree()) when possible.
+The current implementation in `rings/big_oh.py` imports a large number of types, which slows down the Sage startup.  Elements should implement their own big_oh() methods, and the function `O(x)` should simply call `x.big_oh()`.
williamstein commented 13 years ago
comment:9

Tom Boothby is currently working on this. See also #11726 and #11729.

williamstein commented 13 years ago

Changed keywords from none to sd32

boothby commented 13 years ago

Changed author from Timo Kluck to Tom Boothby

boothby commented 13 years ago

Dependencies: #11726, #11719

boothby commented 13 years ago

apply only this patch

boothby commented 13 years ago
comment:12

Attachment: trac_10271.patch.gz

tkluck commented 11 years ago
comment:13

I could have reviewed this sooner, sorry about that. However, the ticket's dependencies are still open, so maybe we should still wait for them to be accepted?

This doesn't apply to v5.4 anymore. I also couldn't easily fix that myself. I'll set the status to needs_work.

tkluck commented 11 years ago

Changed reviewer from Mariah Lenox to Timo Kluck