Opened 2 months ago

Closed 2 months ago

#33963 closed defect (fixed)

wrong sign for symbolic Legendre polynomial at 0

Reported by: gh-DaveWitteMorris Owned by:
Priority: major Milestone: sage-9.7
Component: symbolics Keywords: parity, sign error, pynac
Cc: Merged in:
Authors: Dave Morris Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: a3608a4 (Commits, GitHub, GitLab) Commit: a3608a41a27c55eed9de19c40fb85c9c84db32db
Dependencies: #33962 Stopgaps:

Status badges

Description

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

Change History (10)

comment:1 Changed 2 months ago by gh-DaveWitteMorris

  • Branch set to public/33963

comment:2 Changed 2 months ago by gh-DaveWitteMorris

  • Authors set to Dave Morris
  • Commit set to e29fdaf10f5c6a9bec8d9737d13e9c78ca30393a
  • Dependencies set to #33962
  • Status changed from new to needs_review

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

comment:3 Changed 2 months ago by gh-DaveWitteMorris

  • Branch changed from public/33963 to public/33963r1

comment:4 Changed 2 months ago by gh-DaveWitteMorris

  • Commit changed from e29fdaf10f5c6a9bec8d9737d13e9c78ca30393a to d967fb0b73513b9ea3ff3d7bff6b1f5c7de0e32a

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


New commits:

d967fb0trac 33963: fix legendre polynomial

comment:5 Changed 2 months ago by tscrim

LGTM modulo the doc change:

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

comment:6 Changed 2 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

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

comment:7 Changed 2 months ago by gh-DaveWitteMorris

  • Branch changed from public/33963r1 to public/33963r2

comment:8 Changed 2 months ago by gh-DaveWitteMorris

  • Commit changed from d967fb0b73513b9ea3ff3d7bff6b1f5c7de0e32a to a3608a41a27c55eed9de19c40fb85c9c84db32db

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

comment:9 Changed 2 months ago by gh-DaveWitteMorris

  • Status changed from needs_review to positive_review

comment:10 Changed 2 months ago by vbraun

  • Branch changed from public/33963r2 to a3608a41a27c55eed9de19c40fb85c9c84db32db
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.