Opened 5 years ago

Closed 2 years ago

#18386 closed defect (fixed)

Doctests for: fix polylog evalf

Reported by: rws Owned by:
Priority: major Milestone: sage-8.1
Component: symbolics Keywords: pynac special
Cc: fbissey Merged in:
Authors: Ralf Stephan Reviewers: Paul Masson, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 59d4b29 (Commits) Commit: 59d4b29e5e417a12c9f5cf1bdc58c1fdd810da42
Dependencies: #22969, #23077, #23134 Stopgaps:

Description (last modified by rws)

The polylog function (from Pynac) treats 1.0 like 1 and does not immediately evalf with some numeric arguments.

sage: polylog(2,1)
1/6*pi^2
sage: polylog(2.,1)
1.64493406684823
sage: polylog(2,1.0)
1/6*pi^2
sage: polylog(2,0.9)
polylog(2, 0.900000000000000)
sage: _.n()
TypeError: cannot evaluate symbolic expression numerically

What makes polylog different is that the Sage polylog has no special value logic and calls Pynac's Li_eval for everything. This handles special values (incorrectly if an arg is FP) and sends everything else back with .hold(). So you need N() to get FP results that are not special. With FP Pynac is called which then calls Sage/mpmath. But this then bombs with FP args.

Change History (30)

comment:1 Changed 5 years ago by rws

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. Developers acknowledge bug.

comment:2 Changed 4 years ago by rws

  • Description modified (diff)
  • Milestone changed from sage-6.7 to sage-6.8
  • Summary changed from polylog quirks to implement polylog completely in Sage

comment:3 Changed 3 years ago by rws

  • Description modified (diff)
  • Milestone changed from sage-6.8 to sage-7.3
  • Summary changed from implement polylog completely in Sage to fix polylog evalf

comment:4 Changed 3 years ago by rws

  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

comment:5 Changed 3 years ago by paulmasson

  • Dependencies set to #21034
  • Reviewers set to Paul Masson
  • Status changed from new to needs_review

comment:6 follow-up: Changed 3 years ago by paulmasson

  • Authors set to Ralf Stephan
  • Milestone changed from sage-7.3 to sage-7.4
  • Status changed from needs_review to needs_work

Ralf, there was a typo in the description of #21034 pointing to #18368 rather than this ticket.

I'm finding this behavior odd:

sage: polylog(2,.9999999999999999)
1.64493406684822
sage: polylog(2,1.)
NaN - NaN*I

This doesn't look fixed upstream to me: evalf needs to return a numeric value.

The doctest in functions/log.py captures the current behavior, but needs to be updated before this ticket can be closed.

comment:7 Changed 3 years ago by rws

Polylog has a branch point/discontinuity at arguments (2,1). Anyway, we're just taking what arb is giving us:

sage: RBF=ComplexBallField(53)
sage: RBF(1.5).polylog(RBF(2.5))
[2.27833425640 +/- 2.25e-12] + [-0.61016023975 +/- 3.09e-12]*I
sage: RBF(.99999999).polylog(RBF(2.))
[1.6449338726414 +/- 9.23e-14] + [+/- 2.46e-13]*I
sage: RBF(1.).polylog(RBF(2.))
nan + nan*I
sage: RBF(1.00000001).polylog(RBF(2.))
[1.644934261055 +/- 1.08e-13] + [-3.1416e-8 +/- 3.19e-13]*I
sage: polylog(2,1)
1/6*pi^2

This is very sensible because if you need the exact value say so, and if you need an inexact value to 53 (or any) digits the result cannot be given in that precision so it's NaN. I'll concede that, similar to #19439 it may be desired to return the symbolic NaN.

Last edited 3 years ago by rws (previous) (diff)

comment:8 in reply to: ↑ 6 Changed 3 years ago by rws

  • Authors Ralf Stephan deleted
  • Milestone changed from sage-7.4 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

Replying to paulmasson:

I'm finding this behavior odd:

sage: polylog(2,1.)
NaN - NaN*I

This doesn't look fixed upstream to me: evalf needs to return a numeric value.

Evalf does return a numeric value, too:

sage: polylog(2,1.0).n()
NaN - NaN*I
sage: type(_)
<type 'sage.rings.complex_number.ComplexNumber'>

so, together with my previous answer I think this ticket can be closed.

comment:9 Changed 3 years ago by rws

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-8.0
  • Status changed from needs_review to needs_work

That was badly mistyped and misinterpreted by me, so back to the issue.

comment:10 Changed 3 years ago by rws

Note also

sage: polylog(3,RBF(1.1))      # there is a polylog RBF member!
1.37625299668538 - 0.0142691615444952*I
sage: dilog(RBF(1.1))
1.96199910130557 - 0.299425760685590*I

comment:11 Changed 3 years ago by rws

  • Branch set to u/rws/fix_polylog_evalf

comment:12 Changed 3 years ago by rws

  • Branch changed from u/rws/fix_polylog_evalf to u/rws/18386-1

comment:13 Changed 3 years ago by rws

  • Commit set to 5e182f34798ac3d092aa3e85e5b2fdac4226ce7a
  • Dependencies changed from #21034 to pynac-0.7.7

I believe this is fixed in Pynac master. The tests now also include ball field elements and all open issues from #19906.


New commits:

5e182f318386: doctest fix for dilog/polylog numerics

comment:14 Changed 3 years ago by rws

  • Branch changed from u/rws/18386-1 to u/rws/18386-2

comment:15 Changed 3 years ago by rws

  • Authors set to Ralf Stephan
  • Commit changed from 5e182f34798ac3d092aa3e85e5b2fdac4226ce7a to 51aeae38a87d568034d69a310b680c50146179da
  • Dependencies changed from pynac-0.7.7 to #22969
  • Status changed from needs_work to needs_review

New commits:

51aeae318386: doctest dilog/polylog fixes

comment:16 Changed 2 years ago by rws

  • Status changed from needs_review to needs_work

Fails tests with reduced precision.

comment:17 Changed 2 years ago by git

  • Commit changed from 51aeae38a87d568034d69a310b680c50146179da to 074034a15c082ede5641bd73d4b1a6cb9dfefb9c

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

074034aMerge branch 'develop' into t/18386/18386-2

comment:18 Changed 2 years ago by rws

  • Report Upstream changed from Fixed upstream, in a later stable release. to Not yet reported upstream; Will do shortly.

comment:19 Changed 2 years ago by rws

  • Dependencies changed from #22969 to #22969, #23077, pynac-0.7.8

We depend on #23077 and pynac-0.7.8 for improved handling of complex floats.

comment:20 Changed 2 years ago by rws

  • Dependencies changed from #22969, #23077, pynac-0.7.8 to #22969, #23077, #23134
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to N/A

comment:21 Changed 2 years ago by rws

  • Status changed from needs_work to needs_review
  • Summary changed from fix polylog evalf to Doctests for: fix polylog evalf

Now that all dependencies are merged, let the patchbots test it and please review.

comment:22 Changed 2 years ago by jhpalmieri

See #23565 for some changes to the behavior of polylog. Are those changes improvements?

comment:23 follow-up: Changed 2 years ago by dimpase

The biggest mystery here to me is how pynac is being able to use arb...

comment:24 Changed 2 years ago by dimpase

  • Dependencies changed from #22969, #23077, #23134 to #22969, #23077, #23134, #23565
  • Milestone changed from sage-8.0 to sage-duplicate/invalid/wontfix
  • Reviewers changed from Paul Masson to Paul Masson, Dima Pasechnik
  • Status changed from needs_review to positive_review

the doctests added here, with correct (no NaNs!) outputs, are being added to #23565

comment:25 in reply to: ↑ 23 ; follow-up: Changed 2 years ago by rws

Replying to dimpase:

The biggest mystery here to me is how pynac is being able to use arb...

Pynac calls Sage's arb interface via the Python C API.

comment:26 in reply to: ↑ 25 Changed 2 years ago by dimpase

  • Cc fbissey added

Replying to rws:

Replying to dimpase:

The biggest mystery here to me is how pynac is being able to use arb...

Pynac calls Sage's arb interface via the Python C API.

I see, thanks, this explains why libpynac is not linked to libarb, contrary to what one would expect.

comment:27 Changed 2 years ago by rws

  • Dependencies changed from #22969, #23077, #23134, #23565 to #22969, #23077, #23134

comment:28 Changed 2 years ago by fbissey

  • Branch changed from u/rws/18386-2 to u/fbissey/18386-2
  • Commit changed from 074034a15c082ede5641bd73d4b1a6cb9dfefb9c to 59d4b29e5e417a12c9f5cf1bdc58c1fdd810da42

Just rebasing. Nothing changed.


New commits:

59d4b29Merge branch 'develop' into 18386-2

comment:29 Changed 2 years ago by dimpase

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-8.1

comment:30 Changed 2 years ago by vbraun

  • Branch changed from u/fbissey/18386-2 to 59d4b29e5e417a12c9f5cf1bdc58c1fdd810da42
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.