Opened 5 years ago

Closed 5 years ago

#21644 closed defect (fixed)

Bug in derivatives of some functions to the index var

Reported by: rws Owned by:
Priority: major Milestone: sage-7.4
Component: symbolics Keywords:
Cc: Merged in:
Authors: Carlos R. Mafra Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 2decb06 (Commits, GitHub, GitLab) Commit: 2decb06009ddcc2d3f5c202b6b947ac90b8e9755
Dependencies: Stopgaps:

Status badges

Description (last modified by rws)

sage: gen_laguerre(n,a,x).diff(a)
-gen_laguerre(n - 1, a + 1, x)
sage: gen_laguerre(n,a,x).diff(x)
-gen_laguerre(n - 1, a + 1, x)

sage: derivative(hermite(n,x),x)
2*n*hermite(n - 1, x)
sage: derivative(hermite(n,x),n)
2*n*hermite(n - 1, x)

so something is badly wrong with recognizing the diff. parameter. Note only hermite is implemented in Pynac.

Attachments (1)

0001-Fix-derivative-of-gen_laguerre.patch (2.1 KB) - added by mafra 5 years ago.
Patch to fix issue with gen_laguerre derivative

Download all attachments as: .zip

Change History (13)

comment:1 Changed 5 years ago by rws

  • Description modified (diff)

comment:2 Changed 5 years ago by mafra

I have a patch for this issue (at least for gen_laguerre, my copy of sage doesn't have a derivative method for hermite).

The bug was in handling the diff_param choice, the 'else' branch had the return value for when diff_param was x, but that means that anything that didn't match the ifs before would return the derivative wrt x from the 'else' branch.

The attached patch fixes this (and adds proper handling to laguerre as well)

Changed 5 years ago by mafra

Patch to fix issue with gen_laguerre derivative

comment:3 Changed 5 years ago by tscrim

@mafra you should create and post a git branch of your patch to this ticket. Then one that is done, you can put your (real) name as the author and set it to needs review.

Last edited 5 years ago by tscrim (previous) (diff)

comment:4 Changed 5 years ago by mafra

  • Branch set to u/mafra/bug_in_derivatives_of_some_functions_to_the_index_var

comment:5 Changed 5 years ago by mafra

  • Authors set to Carlos R. Mafra
  • Commit set to e6311e0d7ff6ee6a5d01164beb0917422b3a6112
  • Status changed from new to needs_review

New commits:

e6311e0Fix derivative of gen_laguerre wrt non-implemented variables

comment:6 follow-up: Changed 5 years ago by rws

That is fine, thanks. Can you please add a doctest for the fixed behaviour? We add tests for every branch of possible computation.

BTW Pynac fixes also get Python doctests, partly in the https://github.com/pynac/sage branches (which get merged with the Pynac upgrade ticket into Sage), and partly with separate Sage tickets.

comment:8 Changed 5 years ago by rws

However, see #21655 for a later enhancement.

comment:9 Changed 5 years ago by git

  • Commit changed from e6311e0d7ff6ee6a5d01164beb0917422b3a6112 to 2decb06009ddcc2d3f5c202b6b947ac90b8e9755

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

5b1b51bFix derivative of gen_laguerre wrt non-implemented variables
24b25a4Add doctest to trac 21644
2decb06Merge branch 'u/mafra/bug_in_derivatives_of_some_functions_to_the_index_var' of git://trac.sagemath.org/sage into t/21644/bug_in_derivatives_of_some_functions_to_the_index_var

comment:10 in reply to: ↑ 6 Changed 5 years ago by mafra

Replying to rws:

That is fine, thanks. Can you please add a doctest for the fixed behaviour? We add tests for every branch of possible computation.

Ok, I did that now. But I was not sure if I should have rebased the new patch into the previous one or not, and when I decided to push I got an error message saying that I needed to 'pull' first. After pulling an automatic merge was done, so now there are three commits in this branch.

Should I have rebased the doctest patch into the previous one? Would that require a 'forced' git push?

comment:11 Changed 5 years ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

The merge commit would not have been necessary if you had done git co t/21644/bug_in_derivatives_of_some_functions_to_the_index_var on your machine, added the doctest commt, and finally git trac push. But it's no heavy disaster.

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/mafra/bug_in_derivatives_of_some_functions_to_the_index_var to 2decb06009ddcc2d3f5c202b6b947ac90b8e9755
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.