-
-
Notifications
You must be signed in to change notification settings - Fork 559
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
Make log gamma symbolic #10075
Comments
Changed keywords from none to sd35.5 |
comment:3
Sage gives this error message on startup:
with this change in functions/other.py:
|
comment:4
More precisely,
causes this failure. |
comment:5
It works if you drop the |
comment:6
Weird. So what about things like
? |
Attachment: trac_10075_log_gamma.patch.gz symbolic log_gamma (with modification of functions.rst in case merged before #9130) |
symbolic log_gamma (with modification of functions.rst in case merged after #9130) |
Author: Karen T. Kohl |
comment:7
Attachment: trac_10075_log_gamma_without_functions.rst.patch.gz Load one of the above two patches depending on whether the functions.rst documentation file has been modified already (as in the combined patch for #9130). The second patch file above (without functions.rst) was edited by hand from the first. |
comment:8
Burcin points out that
is not good. Sage itself does
but Wolfram Alpha says
so the branches seem to differ even there. |
comment:9
Since #9130 has positive review: Apply only attachment: trac_10075_log_gamma_without_functions.rst.patch. One might think that Burcin's comment about In fact,
Apparently this is how MPFR deals with this function. So maybe all is well? |
This comment has been minimized.
This comment has been minimized.
comment:10
I mean, for this ticket. Though we should not claim that it is evaluated by Ginac, because it isn't (all the above is in Sage with or without this patch). Believe it or not:
I'm cc:ing Paul Z. just to confirm that this is intended MPFR behavior. We should then open another ticket to make sure to use mpmath or ginac or something to get complex answers. We currently somehow use PARI to get the complex versions.
|
Reviewer: Karl-Dieter Crisman |
comment:11
Ok, here we go.
So both give
I don't see anything holding this up except cosmetics. I'll try to make a refreshed patch momentarily. |
comment:12
Okay, I've been messing with this for too long today.
See also #10072, where a lot of the numerical evaluation was fixed. Anyway, updated patch with more explanation and other information coming up. It needs light review; no code was changed, only doctests. I'm not sure I like the last doctest either
What is the conjugate of plus infinity? But I'll leave it for now, just to document it, unless someone has an objection, since we have in vanilla Sage
I've opened #12521 for the evaluation at negative input with even ceiling function issue (i.e., |
Changed author from Karen T. Kohl to Karen T. Kohl, Karl-Dieter Crisman |
comment:13
I'm marking this as 'needs review', because I did change a fair number of tests. This definitely depends on #9130 because of some doc fixes. Also, I am marking this as depending on #12507, because I don't want to bother fixing that doctest if no one else is either. However, I don't really care either way. |
comment:14
The latest patch attachment: trac_10075.patch failed to apply on top of 5.0.beta4 with this patch queue:
the failure seems to be in
Here is
|
Changed reviewer from Karl-Dieter Crisman to Karl-Dieter Crisman, Benjamin Jones |
comment:15
Makes sense, since beta is in that list now. I was not careful enough about the dependencies, I guess. Coming up. |
Attachment: trac_10075.patch.gz Apply only this patch |
This comment has been minimized.
This comment has been minimized.
comment:16
Okay, all should be well now? Sorry about that. |
comment:17
OK! All looks good now. The patch now applies cleanly to 5.0.beta4 on top of the patch queue in my last comment. I've tested everything in |
comment:18
Actually, one test did fail, but I don't think it's due to this patch (right?)
I haven't seen that failure before. |
Merged: sage-5.0.beta7 |
Changed author from Karen T. Kohl, Karl-Dieter Crisman to Karen Kohl, Karl-Dieter Crisman |
Currently, there is no way to send
log_gamma
to Maxima, for instance. This can be fixed by following the models in the functions/ directory; it should be possible to make it a GinacFunction. Before doing so, though, one will have to resolve #10072, since the evaluation will be wrong (?) otherwise.Apply only attachment: trac_10075.patch.
Depends on #12507
Depends on #9130
CC: @sagetrac-ktkohl @benjaminfjones
Component: symbolics
Keywords: sd35.5
Author: Karen Kohl, Karl-Dieter Crisman
Reviewer: Karl-Dieter Crisman, Benjamin Jones
Merged: sage-5.0.beta7
Issue created by migration from https://trac.sagemath.org/ticket/10075
The text was updated successfully, but these errors were encountered: