search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

conversion of polygamma to fricas #31853

Closed fchapoton closed 3 years ago

fchapoton commented 3 years ago

Related:

CC: @mantepse @slel

Component: interfaces: optional

Keywords: fricas

Author: Frédéric Chapoton

Branch/Commit: c77da15

Reviewer: Martin Rubey, Frédéric Chapoton

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

fchapoton commented 3 years ago

Commit: 9d86cb9

fchapoton commented 3 years ago

New commits:

3c04187Fixing single function and variable with solution_dict solve() corner case.
0b17b06Merge branch 'public/symbolics/solution_dict_list-31452' of ssh://trac.sagemath.org:22/sage into essai
9d86cb9conversion of polygamma to fricas
fchapoton commented 3 years ago

Branch: u/chapoton/31853

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7a6a6bfconvert polygamma to fricas
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9d86cb9 to 7a6a6bf

mantepse commented 3 years ago
comment:4

Could you add polygamma, too, and also include a test along the following lines to make sure that the functions really are the same (unfortunately, both fricas and sage have gaps in the implementation, in different aspects)?

sage: fricas.digamma(1.0)
- 0.5772156649_0153286061
sage: psi(1.0)
-0.577215664901533
sage: fricas.polygamma(1, 1.0)
1.6449340668482269
sage: psi(1, 1.0)
psi(1, 1.00000000000000)
sage: psi(1, 1)
1/6*pi^2
sage: psi(1, 1).n()
1.64493406684823

This might fit with the test in lines 1465-- in fricas.py.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

8c36359add doctests for fricas digamma and polygamma
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 7a6a6bf to 8c36359

fchapoton commented 3 years ago
comment:6

polygamma is added already. Did you mean something else ?

I have added your suggested doctests.

mantepse commented 3 years ago

Changed branch from u/chapoton/31853 to u/mantepse/31853

mantepse commented 3 years ago

Changed branch from u/mantepse/31853 to u/chapoton/31853

mantepse commented 3 years ago
comment:8

Oh, sorry, I overlooked this. I noticed that we can also should translate euler_gamma, I took the liberty to add this, too. If you are happy with this, please set the ticket to positive review on my behalf.

mantepse commented 3 years ago

Changed branch from u/chapoton/31853 to u/mantepse/31853

slel commented 3 years ago

Changed commit from 8c36359 to c77da15

slel commented 3 years ago

Description changed:

--- 
+++ 
@@ -1 +1,4 @@
+Related:

+- #31858: Fix translation of univariate Gamma from FriCAS
+- #25597: Translate bivariate Gamma from FriCAS
slel commented 3 years ago

New commits:

c77da15add translation from euler_gamma
mantepse commented 3 years ago

Reviewer: Martin Rubey

fchapoton commented 3 years ago
comment:12

ok, looks good. Can we set to positive ?

mantepse commented 3 years ago

Changed reviewer from Martin Rubey to Martin Rubey, Frédéric Chapoton

mantepse commented 3 years ago
comment:14

Could you check https://github.com/sagemath/sage-prod/issues/31849, too?

mantepse commented 3 years ago
comment:15

I also fixed #28647 now. I'd be very grateful for a review, so things don't break again.

vbraun commented 3 years ago

Changed branch from u/mantepse/31853 to c77da15