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

fix: gamma functions #943

Merged
merged 3 commits into from
Mar 7, 2025
Merged

fix: gamma functions #943

merged 3 commits into from
Mar 7, 2025

Conversation

wassup05
Copy link
Contributor

@wassup05 wassup05 commented Mar 6, 2025

This PR tries to address #898

The problem was in the interface gpx (meant for internal use only) which had two procedures for x, p are real and x is real, p is an integer, p being > 0 in all cases

Everything was fine with the second procedure but in the first one it is required that (Fast and accurate evaluation of a
generalized incomplete gamma function
) when x < 0, p be an integer which wasn't actually programmed in the procedure itself but was being taken care of as and when requiring to call gpx in such a case. like so https://github.com/fortran-lang/stdlib/blob/master/src/stdlib_specialfunctions_gamma.fypp#L1078-L1099

And hence this was not found earlier as the chunk of code was in the case when x<0 and p<p_lim(x) (check Algorithm 3) which is now unreachable, hence removed

Copy link
Member

@perazz perazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the above suggestion @wassup05. Thank you for the fix.

As an additional request, it would be great to see gamma functions a little better documented in the docs files, so the lazy user doesn't have to search into the source code to understand what ranges and types they work with.

@wassup05
Copy link
Contributor Author

wassup05 commented Mar 6, 2025

Thank you @perazz for your review, I have added your suggestion in the latest commit. Do you mean docs of "in source" comment markup? Because the functions are quite well documented in the spec

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

Copy link
Contributor

@jalvesz jalvesz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @wassup05

@perazz
Copy link
Member

perazz commented Mar 7, 2025

With 3 approvals, I will merge this PR. Thank you @wassup05 for the PR, and @jalvesz @jvdp1 for the reviews.

@perazz perazz merged commit d2767cf into fortran-lang:master Mar 7, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants