Opened 5 years ago
Closed 4 years ago
#16201 closed enhancement (fixed)
default precision for all series (symbolic, power, Laurent)
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  algebra  Keywords:  series precision 
Cc:  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Clemens Heuberger 
Report Upstream:  N/A  Work issues:  
Branch:  0d7da1b (Commits)  Commit:  0d7da1b7f690134eb9109e7062d06df59a5e3ab3 
Dependencies:  Stopgaps: 
Description (last modified by )
Both Laurent and power series ring elements have a default precision of 20. OTOH, symbolic series expansions need the precision set:
sage: R.<t> = LaurentSeriesRing(QQ) sage: 1/(1t) 1 + t + t^2 + t^3 + t^4 + t^5 + t^6 + t^7 + t^8 + t^9 + t^10 + t^11 + t^12 + t^13 + t^14 + t^15 + t^16 + t^17 + t^18 + t^19 + O(t^20) sage: var('x') x sage: ex=1/(1x) sage: ex.series(x) ... TypeError: series() takes exactly 2 positional arguments (1 given)
This ticket proposes to unify the behaviour by allowing the following:
sage: ex.series(x) 1 + 1*x + 1*x^2 + 1*x^3 + 1*x^4 + 1*x^5 + 1*x^6 + 1*x^7 + 1*x^8 + 1*x^9 + 1*x^10 + 1*x^11 + 1*x^12 + 1*x^13 + 1*x^14 + 1*x^15 + 1*x^16 + 1*x^17 + 1*x^18 + 1*x^19 + Order(x^20)
As this is about reducing surprise for the user both algebra and symbolic series default precision should have the same value, so the ticket should also implement such a global value.
This ticket also deprecates the use of LaurentSeriesRing_generic.set_default_prec
as it contradicts the immutability of parents and conflicts with the caching of LaurentSeriesRings?.
Change History (23)
comment:1 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:2 Changed 5 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:3 Changed 4 years ago by
 Branch set to u/rws/default_precision_for_symbolic_power_series_from_sr_series__
comment:4 Changed 4 years ago by
 Commit set to 5fc4999530c6462e560b07dcb2c99463ae6a4a55
 Status changed from new to needs_review
 Summary changed from default precision for symbolic power series from SR.series() to default precision for all series (symbolic, power, Laurent)
comment:5 Changed 4 years ago by
I do not think that order=1
is a good default value, as it changes previous behaviour:
Prior to this patch:
sage: f = sin(x)^(2) ....: f.series(x, 1) 1*x^(2) + Order(1/x)
After this patch:
sage: f = sin(x)^(2) ....: f.series(x, 1) 1*x^(2) + 1/3 + 1/15*x^2 + 2/189*x^4 + 1/675*x^6 + 2/10395*x^8 + 1382/58046625*x^10 + 4/1403325*x^12 + 3617/10854718875*x^14 + 87734/2292899734125*x^16 + 349222/80596287646875*x^18 + Order(x^20)
Furthermore, I'd like to see doctests testing the new behaviour.
Finally, sage.misc.defaults
could be added to the reference manual such that the documentation of all three modified methods (symbolic, power, Laurent) could link to that place for setting the default precision.
comment:6 Changed 4 years ago by
 Commit changed from 5fc4999530c6462e560b07dcb2c99463ae6a4a55 to b3cb45774f2f91f75ea070ffac22a826c7cff396
Branch pushed to git repo; I updated commit sha1. New commits:
b3cb457  16201: change keyword default; add doctests; add misc.defaults to doctree and point to it

comment:7 followup: ↓ 8 Changed 4 years ago by
Done. Unfortunately this is Cython so I had to pick a value that signals default, changed to 65535 now.
Please add your realname to Reviewers.
comment:8 in reply to: ↑ 7 Changed 4 years ago by
 Reviewers set to Clemens Heuberger
 Status changed from needs_review to needs_work
Replying to rws:
Done. Unfortunately this is Cython so I had to pick a value that signals default, changed to 65535 now.
I think that replacing int order=65535
by order=None
would lead to clearer code at a negligible performance penalty.
Further comments:
 The docstrings of
series_precision
andset_series_precision
should haveEXAMPLE
sections.  The parameter
default_prec
of the Laurent series classes should be explained; the only explanation now is indef default_prec(self)
which should be rephrased anyway ("Set ..." is not correct).  I'd like to see doctests showing examples on how to construct series with different values for
set_series_precision
:meth:`set_series_precision`
is a dead link.
comment:9 Changed 4 years ago by
 Commit changed from b3cb45774f2f91f75ea070ffac22a826c7cff396 to 3b682e2a637b58323979f8fca2f97e470a4c2436
Branch pushed to git repo; I updated commit sha1. New commits:
3b682e2  16201: misc.defauts doctests; deprecate LaurentSeriesRing.set_default_prec; cosmetics

comment:10 followup: ↓ 11 Changed 4 years ago by
 Status changed from needs_work to needs_review
The default for order is now None
. The misc
functions now have examples. The description of def default_prec(self)
was corrected. I think the LaurentSeriesRing
method set_default_prec
should be deprecated as is already hinted. Done.
The general improvement of Laurent series documentation is ticket #12259.
I don't think doctests showing examples on how to construct series with different values for set_series_precision
should be added. I don't want to encourage frequent usage of the global setting to set single series precisions.
If the link is dead you should do make docclean; make doc
. It will not suffice to sage docbuild
since a file was added.
comment:11 in reply to: ↑ 10 Changed 4 years ago by
 Status changed from needs_review to needs_work
For now, just three comments, I'll have a closer look later.
File "src/sage/symbolic/expression.pyx", line 3542, in sage.symbolic.expression.Expression.series Failed example: f.series(x) Exception raised: Traceback (most recent call last): File "/local/sage/sage6.5.beta4/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 488, in _run self.compile_and_execute(example, compiler, test.globs) File "/local/sage/sage6.5.beta4/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 850, in compile_and_execute exec(compiled, globs) File "<doctest sage.symbolic.expression.Expression.series[9]>", line 1, in <module> f.series(x) File "sage/symbolic/expression.pyx", line 3584, in sage.symbolic.expression.Expression.series (build/cythonized/sage/symbolic/expression.cpp:20710) x = self._gobj.series(symbol0._gobj, order, 0) TypeError: an integer is required
The reason is that prec
is set but never used.
Replying to rws:
I don't think doctests showing examples on how to construct series with different values for
set_series_precision
should be added. I don't want to encourage frequent usage of the global setting to set single series precisions.
I concur that we should not encourage the actual frequent usage of the global settings. However, I'd prefer it to be at least be covered by a test in a "TESTS" section, just to be sure to avoid small errors as the issue outlined above.
If the link is dead you should do
make docclean; make doc
. It will not suffice tosage docbuild
since a file was added.
I think that the problem is rather the meth
instead of func
.
comment:12 Changed 4 years ago by
The deprecation of set_series_precision
leads to four failing doctests elsewhere:
sage t src/sage/schemes/hyperelliptic_curves/hyperelliptic_padic_field.py ********************************************************************** File "src/sage/schemes/hyperelliptic_curves/hyperelliptic_padic_field.py", line 74, in sage.schemes.hyperelliptic_curves.hyperelliptic_padic_field.HyperellipticCurve_padic_field.local_analytic_interpolation Failed example: x,y,z = HK.local_analytic_interpolation(P,Q) Expected nothing Got: doctest:512: DeprecationWarning: This method is deprecated. See http://trac.sagemath.org/16201 for details. ********************************************************************** sage t src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py ********************************************************************** File "src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py", line 490, in sage.schemes.hyperelliptic_curves.hyperelliptic_generic.HyperellipticCurve_generic.local_coordinates_at_infinity Failed example: x,y = H.local_coordinates_at_infinity(10) Expected nothing Got: doctest:512: DeprecationWarning: This method is deprecated. See http://trac.sagemath.org/16201 for details. ********************************************************************** sage t src/sage/schemes/hyperelliptic_curves/monsky_washnitzer.py ********************************************************************** File "src/sage/schemes/hyperelliptic_curves/monsky_washnitzer.py", line 3580, in sage.schemes.hyperelliptic_curves.monsky_washnitzer.MonskyWashnitzerDifferential.coleman_integral Failed example: w.coleman_integral(P,2*P) Expected: O(5^6) Got: doctest:512: DeprecationWarning: This method is deprecated. See http://trac.sagemath.org/16201 for details. O(5^6) ********************************************************************** sage t src/sage/rings/morphism.pyx ********************************************************************** File "src/sage/rings/morphism.pyx", line 249, in sage.rings.morphism Failed example: R.set_default_prec(10) Expected nothing Got: doctest:1: DeprecationWarning: This method is deprecated. See http://trac.sagemath.org/16201 for details. **********************************************************************
comment:13 Changed 4 years ago by
 Commit changed from 3b682e2a637b58323979f8fca2f97e470a4c2436 to 0fcce825397b78e58a13b0fa5eaa930e7bd17960
Branch pushed to git repo; I updated commit sha1. New commits:
0fcce82  16201: more doctests, fixes; remove deprecated usage

comment:15 followup: ↓ 17 Changed 4 years ago by
 Description modified (diff)
 Status changed from needs_review to positive_review
comment:16 Changed 4 years ago by
Thanks for the review.
comment:17 in reply to: ↑ 15 Changed 4 years ago by
comment:18 Changed 4 years ago by
 Status changed from positive_review to needs_work
This is wrongly indented:
TESTS: Check if changing global series precision does it right::
It should be
TESTS: Check if changing global series precision does it right::
comment:19 Changed 4 years ago by
 Commit changed from 0fcce825397b78e58a13b0fa5eaa930e7bd17960 to 0d7da1b7f690134eb9109e7062d06df59a5e3ab3
Branch pushed to git repo; I updated commit sha1. New commits:
0d7da1b  16201: fix indentation

comment:20 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:21 Changed 4 years ago by
Jeroen, can you please have a quick look again, so that we can finish it?
comment:22 Changed 4 years ago by
 Status changed from needs_review to positive_review
The wrong indentation has been fixed.
comment:23 Changed 4 years ago by
 Branch changed from u/rws/default_precision_for_symbolic_power_series_from_sr_series__ to 0d7da1b7f690134eb9109e7062d06df59a5e3ab3
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
16201: implement global default series precision, accessors