Opened 2 months ago

Closed 2 months ago

#33962 closed defect (fixed)

wrong sign for value of Legendre polynomial at 0

Reported by: gh-DaveWitteMorris Owned by:
Priority: major Milestone: sage-9.7
Component: symbolics Keywords: orthogonal polynomial, legendre polynomial, pynac
Cc: egourgoulhon Merged in:
Authors: Dave Morris Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 8131256 (Commits, GitHub, GitLab) Commit: 8131256c2145a5645e0bb5cf33cf4eff330d0320
Dependencies: Stopgaps:

Status badges

Description

As discussed in this sage-devel thread, legendre_P(n, 0) should be negative when n is congruent to 2 modulo 4, but sagemath returns a positive value:

sage: [legendre_P(n, 0) for n in range(0, 10, 2)]
[1, 1/2, 3/8, 5/16, 35/128]

The correct values are [1, -1/2, 3/8, -5/16, 35/128]. (The signs should alternate when restricted to even values of n.)

This is a pynac bug. It only arises in the code branch where n is an integer variable, so we get the correct values when n is real:

sage: [QQ(legendre_P(RR(n), 0)) for n in range(0, 10, 2)]
[1, -1/2, 3/8, -5/16, 35/128]

Change History (8)

comment:1 Changed 2 months ago by gh-DaveWitteMorris

  • Branch set to public/33962

comment:2 Changed 2 months ago by gh-DaveWitteMorris

  • Commit set to c0114095a3706112f5bda36fa5b6a50ce97b9c94
  • Status changed from new to needs_review

Follow-up ticket: #33963. The patch does not solve the problem when n is a symbolic variable, rather than an explicit integer.


New commits:

c011409trac 33962: fix legendre polynomial

comment:3 Changed 2 months ago by egourgoulhon

  • Cc egourgoulhon added

comment:4 Changed 2 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

LGTM overall. Just a small change to the doc formatting:

-    TESTS::
+    TESTS:
 
-        # verify that :trac:`33962` is fixed
+    Verify that :trac:`33962` is fixed::
+
         sage: [legendre_P(n, 0) for n in range(-10, 10)]
         [0, 35/128, 0, -5/16, 0, 3/8, 0, -1/2, 0, 1,
-        1, 0, -1/2, 0, 3/8, 0, -5/16, 0, 35/128, 0]
+         1, 0, -1/2, 0, 3/8, 0, -5/16, 0, 35/128, 0]

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

Last edited 2 months ago by tscrim (previous) (diff)

comment:5 Changed 2 months ago by git

  • Commit changed from c0114095a3706112f5bda36fa5b6a50ce97b9c94 to 8131256c2145a5645e0bb5cf33cf4eff330d0320

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

8131256reviewer corrections

comment:6 Changed 2 months ago by gh-DaveWitteMorris

Thanks for the review and the corrections!

I will set to positive review when the patchbot is green again.

comment:7 Changed 2 months ago by gh-DaveWitteMorris

  • Status changed from needs_review to positive_review

comment:8 Changed 2 months ago by vbraun

  • Branch changed from public/33962 to 8131256c2145a5645e0bb5cf33cf4eff330d0320
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.