sagemath/sage

wrong sign for symbolic Legendre polynomial at 0

Closed this issue · 15 comments

Trac ticket #33962 fixed the sign of legendre_P(n, 0) when n is an explicit integer. The patch also tried to fix the sign when n is a symbolic integer variable, but the patch does not work. Even with the patch, we have:

sage: n = var("n")
sage: assume(n, "integer")
sage: assume(n, "even")
sage: legendre_P(n, 0)
2^(-n + 2)*gamma(n)/(n*gamma(1/2*n)^2)

This is missing a factor of (-1)^(n/2).

The incorrect value arises because pynac erroneously simplifies the expression pow(_ex_1, n / _ex2) to 1. (The constant _ex_1 is pynac's name for -1.)
The problem seems to be that pynac thinks n / _ex2 is even: printing (n / _ex2).info(info_flags::even) at this point in the code yields 1 (meaning the flag is set).

Depends on #33962

Component: symbolics

Keywords: parity, sign error, pynac

Author: Dave Morris

Branch/Commit: a3608a4

Reviewer: Travis Scrimshaw

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

Dependencies: #33962

Author: Dave Morris

comment:2

The bug is in case info_flags::even of pynac's mul::info method. A product is even if all of the factors are integers, and at least one of the factors is even. However, one of the factors in a mul object is the overall coefficient, and the method neglected to check that this coefficient is an integer.

In the example of the ticket description, pynac represents the expression n/2 as the product 1/2 * n, where 1/2 is the overall coefficient. This coefficient is obviously not an integer.


New commits:

c011409trac 33962: fix legendre polynomial
e29fdaftrac 33963: fix legendre polynomial

Changed branch from public/33963 to public/33963r1

Changed commit from e29fdaf to d967fb0

comment:4

Follow-up: #33964. When n is odd, legendre_P(n, 0) should be 0.


New commits:

d967fb0trac 33963: fix legendre polynomial
comment:5

LGTM modulo the doc change:

-        # verify that :trac:`33963` is fixed
+
+    Verify that :trac:`33963` is fixed::
+
         sage: n = var("n")
comment:6

Once changed, you can set a positive review on my behalf.

Reviewer: Travis Scrimshaw

comment:8

Thanks for the review and corrections of this ticket, too.

I rebased on #33962, and will set to positive review when the patchbot is green again.


New commits:

8131256reviewer corrections
a3608a4trac 33963: fix legendre polynomial

Changed commit from d967fb0 to a3608a4

Changed branch from public/33963r2 to a3608a4