Opened 8 years ago
Closed 8 years ago
#17192 closed defect (fixed)
Update orthogonal polynomials to add errors for negative index
Reported by:  KarlDieter Crisman  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  calculus  Keywords:  beginner, special, functions 
Cc:  Merged in:  
Authors:  Sergey Bykov  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  c20d218 (Commits, GitHub, GitLab)  Commit:  c20d2186f48006de7c48deb264840ff8f5df24cb 
Dependencies:  Stopgaps: 
Description
These accept bad input without warning  the docs are right, apparently, but don't protect against user error very well.
As in that report,
laguerre(1,1) returns 0 in Sage but returns exp(1) in Maple and Mathematica.
Change History (16)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
Branch:  → u/captaintrunky/update_laguerre_poly_to_add_errors 

comment:3 Changed 8 years ago by
Authors:  → Sergey Bykov 

Commit:  → c4ce920fca5f1394eab6e507445b20f9244024af 
Status:  new → needs_review 
New commits:
c4ce920  Added additional check on 'n > 1' condition according to docs.

comment:4 followups: 5 6 Changed 8 years ago by
You disallow Legendre P(n,m,x)
with negative n
, but this is too restrictive, see
sage: pari.pollegendre(3,x) 3/2*x^2  1/2
the implementation in ticket #17151, and http://dlmf.nist.gov/14.9#E3
comment:5 Changed 8 years ago by
comment:6 Changed 8 years ago by
Replying to rws:
You disallow Legendre
P(n,m,x)
with negativen
, but this is too restrictive, seesage: pari.pollegendre(3,x) 3/2*x^2  1/2the implementation in ticket #17151, and http://dlmf.nist.gov/14.9#E3
Does it mean that docs are inconsistent for these functions: legendre_P, legendre_Q, gen_legendre_P, gen_Legendre_Q? All of them have the constraint:
Returns the generalized (or associated) Legendre function of the second kind for integers `n>1`, `m>1`.
or something similar.
comment:7 followup: 8 Changed 8 years ago by
That too. It also means that implementing the more general solution makes the Legendre part of your ticket obsolete. I also want encourage people to review #16813.
comment:8 Changed 8 years ago by
comment:9 Changed 8 years ago by
Commit:  c4ce920fca5f1394eab6e507445b20f9244024af → 799ad05eb267f11c182cc19ed32a0b1ebe345d6b 

Branch pushed to git repo; I updated commit sha1. New commits:
799ad05  Fixes docs and removes additional checks on input for 'legender' functions

comment:10 followup: 11 Changed 8 years ago by
BTW, even Laguerre polynomials can have extensions with negative or noninteger n
, for example mpmath's numeric implementation: https://mpmath.googlecode.com/svn/trunk/doc/build/functions/orthogonal.html#laguerrepolynomials
I'm using this function in #17151 and so the restriction will fall there as well. You can experiment with what mpmath returns like this in Sage, as long as #17151 is not included:
sage: from mpmath import laguerre as lag sage: lag(1/2,0,1) mpf('0.42519582689040547') sage: lag(1,0,1) mpf('0.0') sage: lag(1+I,0,1) mpc(real='2.224699152616044', imag='2.1127345594510065') sage: lag(1,I,1) mpc(real='6.3272150395398353e23', imag='1.0')
comment:11 Changed 8 years ago by
Replying to rws:
BTW, even Laguerre polynomials can have extensions with negative or noninteger
n
, for example mpmath's numeric implementation: https://mpmath.googlecode.com/svn/trunk/doc/build/functions/orthogonal.html#laguerrepolynomials I'm using this function in #17151 and so the restriction will fall there as well. You can experiment with what mpmath returns like this in Sage, as long as #17151 is not included:sage: from mpmath import laguerre as lag sage: lag(1/2,0,1) mpf('0.42519582689040547') sage: lag(1,0,1) mpf('0.0') sage: lag(1+I,0,1) mpc(real='2.224699152616044', imag='2.1127345594510065') sage: lag(1,I,1) mpc(real='6.3272150395398353e23', imag='1.0')
I've played with it a little bit. As far as I understand, there is no way to use negative or noninteger n
with a current implementation. My question is, is it worth it to support it in this particular code? There is some activity on the general solution already, so probably there is no point in doing such modifications?
comment:12 Changed 8 years ago by
What I can say is, our two proposals cause mutual merge conflict, but since no one knows which will be reviewed and included first, there is no good plan. It may take a few weeks until I am free to review your ticket.
comment:13 Changed 8 years ago by
Keywords:  special functions added 

Reviewers:  → Ralf Stephan 
Summary:  Update Laguerre poly to add errors → Update orthogonal polynomials to add errors for negative index 
comment:14 Changed 8 years ago by
Branch:  u/captaintrunky/update_laguerre_poly_to_add_errors → u/rws/update_laguerre_poly_to_add_errors 

comment:15 Changed 8 years ago by
Commit:  799ad05eb267f11c182cc19ed32a0b1ebe345d6b → c20d2186f48006de7c48deb264840ff8f5df24cb 

Status:  needs_review → positive_review 
I removed some lines that prevented the doc being built. The code passes make ptestlong
and is straightforward, so it's positive review.
New commits:
1b5efb9  Merge branch 'u/captaintrunky/update_laguerre_poly_to_add_errors' of trac.sagemath.org:sage into t17192

c20d218  17192: reviewer's patch: superfluous lines in doctests

comment:16 Changed 8 years ago by
Branch:  u/rws/update_laguerre_poly_to_add_errors → c20d2186f48006de7c48deb264840ff8f5df24cb 

Resolution:  → fixed 
Status:  positive_review → closed 
Probably other things in sage/functions/orthogonal_polys.py could use similar upgrades.