UniqueRepresentation issue with PowerSeriesRing

[videlec]

The power series ring (sage.rings.power_series_ring.PowerSeriesRing) inherits from UniqueRepresentation. In the argument of the constructor there is the precision (prec) but it can be modified! Which leads to wrong behavior

sage: R = PowerSeriesRing(QQ, 'x', 3)
sage: R.default_prec()
3
sage: R.set_default_prec(19)
sage: PowerSeriesRing(QQ, 'x', 3).default_prec()
19

Moreover, a function can modify a power series in use

sage: def haha()
....:     PowerSeriesRing(QQ, 'x').set_default_prec(1)
sage: R = PowerSeriesRing(QQ, 'x')
sage: R.default_prec()
20
sage: haha()
sage: R.default_prec()
1

Depends on #31263

CC: @nilesjohnson @simon-king-jena @tscrim @slel

Component: algebra

Keywords: power_series

Author: Michael Jung

Branch/Commit: f38b78e

Reviewer: Travis Scrimshaw

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

[alexjbest]

comment:3

Does anyone know the status of this?

The corresponding behavior for LaurentSeries was deprecated in #16201 (which also mentions power series) and finally removed in #26915 .

So now we are in the situation where PowerSeries and LaurentSeries work quite differently here, I can't see any real reason for this.

[tscrim]

comment:4

+1 on deprecating the set_default_prec for PowerSeriesRing for the same reasons as for LaurentSeriesRing.

[tscrim]

comment:5

Even worse in terms of hidden side effects:

sage: R.<x> = LaurentSeriesRing(QQ,50)
sage: R.default_prec()
50
sage: S = PowerSeriesRing(QQ,50,names='x')
sage: S.set_default_prec(10)
sage: R.default_prec()
10

[nilesjohnson]

comment:6

yes, this should definitely be changed

[embray]

comment:7

Moving all blocker/critical issues from 8.7 to 8.8.

[embray]

comment:8

Moving open critical and blocker issues to the next release milestone (optimistically).

[embray]

comment:9

Ticket retargeted after milestone closed

[mjungmath]

comment:12

I propose to go back to the idea from #16201. In particular, we should handle the whole precision business using a global variable. If desired, one could introduce several global variables (e.g. one for univariate power series and another for multivariate).

I'll upload a branch soon.

[mjungmath]

comment:14

Okay, this is tricky: mostly all doctests heavily rely on the default_prec syntax, which is actually bad. Either we add after each example where the precision has been changed a

sage: set_series_precision(20)  # reset to default

or we have to establish each example with the default precision.

At least, this behavior would be expected with the proposed changes, whereas now it is entirely random to the user.

Alternatively, we come up with an entirely different solution. I am open for suggestions.

[mjungmath]

Branch: u/gh-mjungmath/uniquerepresentation_issue_with_powerseriesring

[mjungmath]

comment:16

So, this my first approach. Since the global variable series_prec keeps track of the multivariate case too (otherwise one runs into problems with the background power series ring), I set the default to 15, as a compromise for now.

If this meets your approval, we have to think what we should do with the failed doctests.

I know, this is a big change. But the current behavior should not be further promoted either.

---

New commits:

d4b84d0

Trac #18416: global precision

[mjungmath]

Changed keywords from none to power_series

[mjungmath]

Commit: d4b84d0

[mjungmath]

comment:38

Okay, this class is used nowhere else. Instead, I have deprecated the method in Nonexact and improved its documentation to clarify the proper usage.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

f806db1

Trac #18416: remove double underscore;__default_prec -> _default_prec

[sagetrac-git]

Changed commit from a13c408 to f806db1

[tscrim]

comment:40

The only things I would change before giving a positive review (with a green patchbot) are the deprecation message to

The method set_default_prec() is deprecated and will be removed in a future version of Sage. The default precision is set during construction.

and this

-The default precision of a power series ring instance stays fixed and
-cannot be changed. To work with different default precisions, we must
-establish new instances instead::
+The default precision of a power series ring stays fixed and
+cannot be changed. To work with different default precision, create
+a new power series ring::

[sagetrac-git]

Changed commit from f806db1 to af42804

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

eecbf2a	Trac #18416: deprecate set_default_prec
0a176d1	Trac #18416: remove double underscore;__default_prec -> _default_prec
af42804	Trac #18416: deprecation message

[mjungmath]

comment:42

There we go. :)

[fchapoton]

comment:43

changing src/bin/sage is not a thing to do

[mjungmath]

comment:44

I merged #31263 to use the sage -b command...

Rebase?

[fchapoton]

comment:45

• you could have used "make build" instead of "sage -b", until this is repaired

• do not rebase, but add Broken sage -b #31263 in the dependency field here above (I did that for you)

• changing the src/bin/sage file make your ticket "unsafe", so patchbots do not test it

[fchapoton]

Dependencies: #31263

[mjungmath]

comment:46

That is good to know. Thank you for pointing this out.

Travis, are you satisfied?

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:47

Yep. LGTM. Thank you.

[vbraun]

comment:48

[dochtml] Traceback (most recent call last):
[dochtml]   File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
[dochtml]     return _run_code(code, main_globals, None,
[dochtml]   File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code
[dochtml]     exec(code, run_globals)
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__init__.py", line 1738, in main
[dochtml]     builder()
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__init__.py", line 345, in _wrapper
[dochtml]     getattr(get_builder(document), 'inventory')(*args, **kwds)
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__init__.py", line 577, in _wrapper
[dochtml]     self._build_everything_except_bibliography(lang, format, *args, **kwds)
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__init__.py", line 563, in _build_everything_except_bibliography
[dochtml]     build_many(build_ref_doc, non_references)
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/__init__.py", line 297, in build_many
[dochtml]     _build_many(target, args, processes=processes)
[dochtml]   File "/home/release/Sage/local/lib64/python3.9/site-packages/sage_setup/docbuild/utils.py", line 289, in build_many
[dochtml]     raise worker_exc.original_exception
[dochtml] OSError: /home/release/Sage/local/lib64/python3.9/site-packages/sage/structure/nonexact.py:docstring of sage.structure.nonexact.Nonexact:6: WARNING: Bullet list ends without a blank line; unexpected unindent.
[dochtml] 
[dochtml]     Note: incremental documentation builds sometimes cause spurious
[dochtml]     error messages. To be certain that these are real errors, run
[dochtml]     "make doc-clean" first and try again.
make[3]: *** [Makefile:1916: doc-html] Error 1

[sagetrac-git]

Changed commit from af42804 to f38b78e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

f38b78e

Trac #18416: docstring bullet point

[mjungmath]

comment:50

Just one character missing...here we go.

[vbraun]

Changed branch from u/gh-mjungmath/uniquerepresentation_issue_with_powerseriesring to f38b78e