Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong sign for value of Legendre polynomial at 0 #33962

Closed
DaveWitteMorris opened this issue Jun 8, 2022 · 9 comments
Closed

wrong sign for value of Legendre polynomial at 0 #33962

DaveWitteMorris opened this issue Jun 8, 2022 · 9 comments

Comments

@DaveWitteMorris
Copy link
Member

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]

CC: @egourgoulhon

Component: symbolics

Keywords: orthogonal polynomial, legendre polynomial, pynac

Author: Dave Morris

Branch/Commit: 8131256

Reviewer: Travis Scrimshaw

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

@DaveWitteMorris
Copy link
Member Author

Branch: public/33962

@DaveWitteMorris
Copy link
Member Author

comment:2

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

@DaveWitteMorris
Copy link
Member Author

Commit: c011409

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2022

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2022

comment:4

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2022

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

8131256reviewer corrections

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2022

Changed commit from c011409 to 8131256

@DaveWitteMorris
Copy link
Member Author

comment:6

Thanks for the review and the corrections!

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

@vbraun
Copy link
Member

vbraun commented Jun 12, 2022

Changed branch from public/33962 to 8131256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants