Upgrade to pynac-0.7.22

[Ralf Stephan]

In the new version:

• fast and working versons of quo_rem and rational decomposition (#25644, #25645)
• fix power inconsistency
• internal fix of another power inconsistency (#25639)
• (x^a)^b --> x^(ab) IF x real, a odd integer, ab integer
• add ex::treesize() as complexity metric (#25643)
• information for packagers

• implement commutative matching; fix GiNaC's matching deficiencies (#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(#21945)
• performance and readability improvements

• correct sinh/cosh/tanh return type (#24299)
• atan2 fixes
• potential memleaks fixed
• fixes in Giac interface
• subs can now substitute numeric terms (#23964)

• fix memleak in in-place PyObject numerics (#24745)
• fix endless computation (#24883)
• Python interface / Py3 fixes and improvements (thx Erik Bray, #24561, #24752, #24522)
• fix internal asin/acos of complex balls
• draw factors out of add^rational (#24768, #25251, #25252)
• exp(f(x)), exp(c*f(x)), f inv. hyperbolic simplifications (#24841)
• always extend trig/hyperbolic functions to complex domain (#24428)
• extensive code readability improvements

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

[Ralf Stephan]

New commits:

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

[Jeroen Demeyer]

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

[Ralf Stephan]

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

[Jeroen Demeyer]

Replying to rws:

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.

[Ralf Stephan]

The issue is no longer there. Please review.

[Jeroen Demeyer]

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

[Ralf Stephan]

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.

[Ralf Stephan]

See also ​https://github.com/fredrik-johansson/arb/issues/198

[Ralf Stephan]

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

[Ralf Stephan]

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

[Ralf Stephan]

See ​https://github.com/fredrik-johansson/arb/pull/210

[Jeroen Demeyer]

Replying to rws:

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))

[Ralf Stephan]

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

[Ralf Stephan]

Replying to rws:

See ​https://github.com/fredrik-johansson/arb/pull/210

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

[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

[Ralf Stephan]

New release incoming.

[git]

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

​ae5eac7

24838: version/chksum

[Ralf Stephan]

0.7.17 introduced a bug in exp(ex*log(x)) simplification, and we need a new release, anyway.

[git]

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

​79d54ea	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​75b2f85	fix doctests
​8878283	24838: version/chksum

[Jeroen Demeyer]

This looks fishy:

-            sage: SR(CDF(1/2)).arccosh()  # rel tol 1e-15
+            sage: SR(CDF(1/2)).arccosh() # abs tol 1e-15

[Jeroen Demeyer]

The analogous thing as 9 should be fixed for pure imaginary results:

sage: CDF(1/2).arccosh()
1.0471975511965979*I
sage: SR(CDF(1/2)).arccosh()
-6.372599439108525e-21 + 1.0471975511965979*I

Then you also revert 29

[Ralf Stephan]

Replying to jdemeyer:

The analogous thing as 9 should be fixed for pure imaginary results:

sage: CDF(1/2).arccosh()
1.0471975511965979*I
sage: SR(CDF(1/2)).arccosh()
-6.372599439108525e-21 + 1.0471975511965979*I

As explained in 10 missing FP functionality is gained using arb. But there was that bug in arb which is now fixed. Consequentially we need that fix or a recent arb to get the right result, see 14.

Then you also revert 29

Will re-revert.

[Ralf Stephan]

I confirm that this works with ​https://github.com/fredrik-johansson/arb/commit/7afd3bfaf1697739751d96d3665ef83e78c76820 applied, which is in arb-2.13.0 (#24927).

[git]

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

​8a2217a	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​3efba1a	fix doctest

[Ralf Stephan]

Some recent Sage commit makes doctests (involving powers of I) fail together with pynac-0.7.17 so we'll need a fix in 0.7.20.

[Ralf Stephan]

Actually it's not our fault: #25458.

[git]

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

​ed36070	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​3278103	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​ca69aef	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​97bf3b7	24838: doctest fixes
​e6b01df	doctest fix
​fdb4210	Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17
​ad07cd5	24838: 0.7.20 version/chksum

[Ralf Stephan]

The dependency is now in. Here a fresh branch with the newest Pynac.

---

New commits:

​5f6c08e	24838: pkg version / chksum / remove old patch
​3b244e9	24838: doctest fixes

[Frédéric Chapoton]

I got "invalid checksum"

[Ralf Stephan]

Please check again, I just now did not get any error with the file linked in the ticket. Did you reload the branch?

[Frédéric Chapoton]

Indeed. Works now. I must have made some mistake in my previous attempt.

[Frédéric Chapoton]

got

sage -t --long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py  # 2 doctests failed
sage -t --long src/sage/symbolic/expression.pyx  # 1 doctest failed

[Frédéric Chapoton]

Doctesting 2 files.
sage -t --long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 153, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions
Failed example:
    F1.asymptotics(p, alpha, 2)
Expected:
    (-3*((2*a^2 - 5*a*b + 2*b^2)*r^2 + (a + b)*r + 3)*((1/3)^(-a)*(1/3)^(-b))^r,
     (1/3)^(-a)*(1/3)^(-b), -3*(2*a^2 - 5*a*b + 2*b^2)*r^2 - 3*(a + b)*r - 9)
Got:
    (-3*((2*a^2 - 5*a*b + 2*b^2)*r^2 + (a + b)*r + 3)*(1/((1/3)^a*(1/3)^b))^r,
     1/((1/3)^a*(1/3)^b),
     -3*(2*a^2 - 5*a*b + 2*b^2)*r^2 - 3*(a + b)*r - 9)
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 2161, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.FractionWithFactoredDenominator.?
Failed example:
    F.asymptotics_multiple(p, alpha, 2, var('r')) # long time
Expected:
    (3*((1/3)^(-a)*(1/3)^(-b))^r*e^(2/3), (1/3)^(-a)*(1/3)^(-b), 3*e^(2/3))
Got:
    (3*(1/((1/3)^a*(1/3)^b))^r*e^(2/3), 1/((1/3)^a*(1/3)^b), 3*e^(2/3))
**********************************************************************
2 items had failures:
   1 of  77 in sage.rings.asymptotic.asymptotics_multivariate_generating_functions

and

Failed example:
    SR(CDF(1/2)).arccosh() #  rel tol 1e-15
Expected:
    1.0471975511965976*I
Got:
    -6.372599439108525e-21 + 1.0471975511965979*I

[Ralf Stephan]

Replying to chapoton:

Failed example:
    SR(CDF(1/2)).arccosh() #  rel tol 1e-15
Expected:
    1.0471975511965976*I
Got:
    -6.372599439108525e-21 + 1.0471975511965979*I

Not confirmed. You apparently don't have the dependency installed, the newest arb.

[git]

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

​814e549

24838: fix to doctests

[Frédéric Chapoton]

The second doctest is still failing for me.

sage: version()
'SageMath version 8.3.beta7, Release Date: 2018-06-23'
sage: package_versions('standard')['arb']
('2.13.0', '2.13.0')

[Ralf Stephan]

Replying to chapoton:

The second doctest is still failing for me.

Can you please give the output of:

sage: CDF(1/2).arccosh()
1.0471975511965979*I
sage: CBF(1/2).arccosh()
[1.047197551196598 +/- 7.94e-16]*I

[Frédéric Chapoton]

sage: CDF(1/2).arccosh()
1.0471975511965979*I
sage: CBF(1/2).arccosh()
[+/- 4.90e-16] + [1.047197551196598 +/- 7.94e-16]*I

[Ralf Stephan]

This is peculiar. I was under the impression that this fix: ​https://github.com/fredrik-johansson/arb/commit/7afd3bfaf1697739751d96d3665ef83e78c76820 would be in arb-2.13.0. I just found out that it was committed Mar 5, while the release was Feb 23. I should have actively propagated that patch into our arb upgrade ticket.

Should I open a separate ticket for the arb patch or just put it into this branch?

I would have noticed earlier but for some reason I don't get the error with 2.13.0.

[Ralf Stephan]

Ah, I had the patch still in the patches directory. New files are not deleted when changing branches.

[git]

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

​6645f79

24838: add arb commit 7afd3afbf

[Ralf Stephan]

This is reviewed by Fredrik in ​https://github.com/fredrik-johansson/arb/pull/210

[Frédéric Chapoton]

This should bump the arb version by .p0

[git]

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

​8e4e081

24838: bump arb version

[Frédéric Chapoton]

Thanks. This should be ready to go.

Launching now "make ptestlong" once again, just to be sure.

[Frédéric Chapoton]

ok, all tests pass

[Ralf Stephan]

Thanks for the review!

[Volker Braun]

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

[Ralf Stephan]

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.

[Ralf Stephan]

Replying to rws:

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 ...".

[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

[Timo Kaufmann]

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?

[Ralf Stephan]

Replying to gh-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.

[Timo Kaufmann]

Replying to rws:

Replying to gh-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.

[Timo Kaufmann]

Replying to 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)

[Jeroen Demeyer]

Replying to gh-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.