Upgrade to pynac-0.7.22

[rwst]

In the new version:

• fast and working versons of quo_rem and rational decomposition (Symbolic quo //, rem %, and quo_rem() #25644, Fast symbolic rational decomposition #25645)

• fix power inconsistency

• internal fix of another power inconsistency (inconsistent powers of symbolic sums #25639)

• (x<sup>a)</sup>b --> x^(ab) IF x real, a odd integer, ab integer

• add ex::treesize() as complexity metric (Fast symbolic complexity metric #25643)

• information for packagers

• implement commutative matching; fix GiNaC's matching deficiencies (Doctests: Symbolic expression matching bug #25168)

• substitute patternless if no wildcards present

• remove some compiler warnings

• update AX_CXX_COMPILE_STDCXX.m4

• fix exp(c*f(x)) simplification

• fix cases(...).subs(...)

• function::info for min/max_symbolic(Symbolic min/max #21945)

• performance and readability improvements

• correct sinh/cosh/tanh return type (Systematic doctest for function return type #24299)

• atan2 fixes

• potential memleaks fixed

• fixes in Giac interface

• subs can now substitute numeric terms (Doctest fix: f.subs(I==...) does not work #23964)

• fix memleak in in-place PyObject numerics (leaking in the symbolic ring #24745)

• fix endless computation (Endless symbolic computation #24883)

• Python interface / Py3 fixes and improvements (thx Erik Bray, py3: fix bugs in pynac with handling Python 3 ints #24561, py3: unhandled exception in pynac #24752, Pynac should check for errors when calling PyObject_RichCompareBool() #24522)

• fix internal asin/acos of complex balls

• draw factors out of add^rational (Powers of symbolic sums inconsistence #24768, Doctest: Certain products cause pynac to deadloop #25251, Doctest: Complex arithmetic/exponentiation hang (or very slow) #25252)

• exp(f(x)), exp(c*f(x)), f inv. hyperbolic simplifications (Doctest exp(c*f(x)) simplification, f inverse hyperbolic #24841)

• always extend trig/hyperbolic functions to complex domain (Numerical evaluation should return a complex number if applicable #24428)

• extensive code readability improvements

https://github.com/pynac/pynac/releases/download/pynac-0.7.22/pynac-0.7.22.tar.bz2

Depends on #24927

CC: @kiwifb @timokau

Component: symbolics

Author: Ralf Stephan

Branch: e324b64

Reviewer: Frédéric Chapoton

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

[rwst]

Branch: u/rws/upgrade_to_pynac_0_7_17

[rwst]

New commits:

e470685	version / chkum / rm patch
d3511ce	24838: doctest fixes

[rwst]

Commit: d3511ce

[rwst]

Author: Ralf Stephan

[jdemeyer]

comment:5

Just a question: is this supposed to fix #24522 too?

[rwst]

comment:6

Yes, that PR is merged. I'll probably add a hotfix for #24883 here as patch, too.

[jdemeyer]

comment:7

Replying to @rwst:

I'll probably add a hotfix for #24883 here as patch, too.

Then I'm setting this to needs_work. If you change your mind, feel free to set this back to needs_review.

[rwst]

comment:8

The issue is no longer there. Please review.

[jdemeyer]

comment:9

This used to return a purely imaginary number, now there is a small real part:

sage: arccosh(x).subs(x=0.9)
-9.66146955461936e-22 + 0.451026811796262*I

[rwst]

comment:10

This is an artifact of conversion to CC:

sage: CC(ComplexBallField(68)(0.9).arccosh())
-9.66146955461936e-22 + 0.451026811796262*I

We use arb with precision+15 and convert back to parent, all to make up for missing or limited (in the calculus sense, not the algebraic, note recent discussion) member functions of parent (RR in this case). Before this pynac version we had specific logic to zero the real part but this was error-prone. IMHO CC conversion of complex balls should be fixed.

[rwst]

comment:11

See also flintlib/arb#198

[rwst]

comment:12

You may also want to know why I'm using arb and not mpfr:

sage: r=srange(-2.5,2.5,.01)
sage: rr = [CC(i) for i in r]
sage: %timeit for x in rr: _=x.arccosh()
100 loops, best of 3: 11.9 ms per loop
sage: rr = [CBF(i) for i in r]
sage: %timeit for x in rr: _=x.arccosh()
1000 loops, best of 3: 649 µs per loop

[rwst]

comment:13

What do you prefer, patching arb or the arb interface?

[rwst]

comment:14

See flintlib/arb#210

[jdemeyer]

comment:15

Replying to @rwst:

You may also want to know why I'm using arb and not mpfr:

Despite what you think, you are not using mpfr, but PARI:

    def arccosh(self):
        """
        Return the hyperbolic arccosine of ``self``.

        EXAMPLES::

            sage: (1+CC(I)).arccosh()
            1.06127506190504 + 0.904556894302381*I
        """
        return self._parent(self.__pari__().acosh())

If you want to compare with something, it should be mpc (in Sage: CC = MPComplexField(53))

[rwst]

comment:16

Interestingly,

sage: arccosh(x).subs(x=MPComplexField(53)(0.9))
...
TypeError: no canonical coercion from Complex Field with 53 bits of precision to Symbolic Ring

[rwst]

comment:17

Replying to @rwst:

See flintlib/arb#210

This is now merged. Do we want that patch in this ticket? If not, anything else?

[sagetrac-git]

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

e85fa6c	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
9f86eb7	24838: pynac-0.7.18 doctest fixes

[rwst]

comment:63

Thanks for the review!

[vbraun]

comment:64

On 32-bit:

File "src/sage/functions/hypergeometric.py", line 148, in sage.functions.hypergeometric
Failed example:
    hypergeometric_U(2, 2, x).series(x == 3, 100).subs(x=1).n()
Expected:
    0.403652637676806
Got:
    0.403651580752398

[rwst]

comment:65

Replying to @vbraun:

On 32-bit:

File "src/sage/functions/hypergeometric.py", line 148, in sage.functions.hypergeometric
Failed example:
    hypergeometric_U(2, 2, x).series(x == 3, 100).subs(x=1).n()
Expected:
    0.403652637676806
Got:
    0.403651580752398

Pynac is not involved in the numerics (hypergeometric_U is not a GinacFunction), so the hypothesis is that the expression before .n() is different with 32bit pynac-0.7.22.

[rwst]

comment:67

Replying to @rwst:

Replying to @vbraun:

On 32-bit:

Pynac is not involved in the numerics (hypergeometric_U is not a GinacFunction), so the hypothesis is that the expression before .n() is different with 32bit pynac-0.7.22.

We will never know. I failed to install 32bit versions of CentOS and ArchLinux in a VM, and I will not try again. There will be a ticket for this doctest failure, and the test marked "known bug (see ...".

[sagetrac-git]

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

5f6c08e	24838: pkg version / chksum / remove old patch
3b244e9	24838: doctest fixes
814e549	24838: fix to doctests
6645f79	24838: add arb commit 7afd3afbf
8e4e081	24838: bump arb version
e324b64	24838: mark 32-bit fail as known bug, see 25688

[sagetrac-git]

Changed commit from 8e4e081 to e324b64

[timokau]

comment:70

This isn't really about this specific ticket (thanks for putting in the work for the upgrade), but is there a reason everybody seems to make seperate "pkg version / chksum" and "doctest fixes" patches?

Shouldn't each commit be one unit of changes for which the doctests still pass?

[rwst]

comment:71

Replying to @timokau:

Shouldn't each commit be one unit of changes for which the doctests still pass?

The only reason I see for this is to make statistics based on number of commits meaningful. By splitting commits into parts (if there is such a natural split, maybe here it's less needed) I make the job of reviewing easier.

[timokau]

comment:72

Replying to @rwst:

Replying to @timokau:

Shouldn't each commit be one unit of changes for which the doctests still pass?

The only reason I see for this is to make statistics based on number of commits meaningful. By splitting commits into parts (if there is such a natural split, maybe here it's less needed) I make the job of reviewing easier.

A much more practical reason is the use of git-bisect or just generally being able to check out some interesting commit and build from it.

[vbraun]

Changed branch from u/rws/24838 to e324b64

[timokau]

comment:74

Replying to @sagetrac-git:

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

6645f79

24838: add arb commit 7afd3afbf

For the future it would greatly simplify packaging if you would add some documentation when adding a patch. A link to the github issue/pr and maybe a sentence about what it does would be sufficient. (The commit is actually 7afd3bfaf)

[timokau]

Changed commit from e324b64 to none

[jdemeyer]

comment:75

Replying to @timokau:

For the future it would greatly simplify packaging if you would add some documentation when adding a patch. A link to the github issue/pr and maybe a sentence about what it does would be sufficient. (The commit is actually 7afd3bfaf)

+1

Patches without any documentation or links should be rejected.