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

Using gamma_inc method not defined in SpecialFunctions minimum version #136

Closed
sethaxen opened this issue Apr 1, 2022 · 8 comments · Fixed by #138
Closed

Using gamma_inc method not defined in SpecialFunctions minimum version #136

sethaxen opened this issue Apr 1, 2022 · 8 comments · Fixed by #138

Comments

@sethaxen
Copy link
Contributor

sethaxen commented Apr 1, 2022

This package sets the lower bound for SpecialFunctions compatibility to v0.8.0. However, it uses the 2-arg gamma_inc method, which was not defined until SpecialFunctions v1.2.0. Since the Julia lower bound for SpecialFunctions v1.2.0 was v1.3, this results in gammacdf erroring for all pre-v1.3 versions of Julia, even though the Julia lower bound on this package is v1.0.

This could be resolved by using the 3-arg method of gamma_inc. It would also be good to add the minimum supported Julia version to the CI.

@devmotion
Copy link
Member

I don't think it's useful to support such old versions of SpecialFunctions at all. There are multiple numerical issues without the recent bugfixes in SpecialFunctions 1.x and 2.x (we backported them to 1.x). So probably it is more appropriate to update the Julia compat.

@sethaxen
Copy link
Contributor Author

sethaxen commented Apr 1, 2022

Since SpecialFunctions v1.2 does not support pre-Julia v1.3 versions at all, simply bumping the SpecialFunctions compat leaves older Julia versions with a version of this package for which gammacdf errors, right? That needs to be fixed first.

@rikhuijzer
Copy link

Since SpecialFunctions v1.2 does not support pre-Julia v1.3 versions at all, simply bumping the SpecialFunctions compat leaves older Julia versions with a version of this package for which gammacdf errors, right? That needs to be fixed first.

Possibly, but how much effort do we want to put into supporting Julia pre 1.6? Julia 1.6 was released more than a year ago.

@sethaxen
Copy link
Contributor Author

sethaxen commented Apr 1, 2022

I did not suggest we should support older Julia versions. The most recent release of this package introduced the 2-arg gamma_inc method, which was a bug for versions of this package that it claims to support. I am merely suggesting we fix that bug before dropping support for those versions, or we are just leaving them with a bug.

Otherwise we coerce downstream dependencies to either all drop support for pre-Julia v1.3 so their CI passes or stop testing their minimum supported Julia version, which is not great.

@devmotion
Copy link
Member

I understand but unfortunately the package will still not work correctly on these older Julia versions. If you use the latest Rmath-reduced versions you will run into numerical issues and errors (reported in StatsFuns and eg in Distributions) if you don't use the latest releases of SpecialFunctions 1 or 2.

@devmotion
Copy link
Member

I pinged the JuliaStats contributors on Slack to get their opinion on whether we should fix the gamma_inc call and live with issues on Julia < 1.3 that were not present in the Rmath versions <= 0.9.15, or if we should fix the compat in the registry (if possible) by making 0.9.16 only available on Julia >= 1.3.

@devmotion
Copy link
Member

Copied from Slack:

Bogumił Kamiński: Do 0.9.16 and 0.9.15 have the same functionality? (except that they have different internals?)

David Widmann: Hmm I assumed so but it seems 0.9.16 containshttps://github.com//pull/67 as well (https://github.com/JuliaStats/StatsFuns.jl/releases/tag/v0.9.16). So maybe it's better to just live with the numerical regressions with older SpecialFunctions/Julia versions, tag a bugfix release, and possibly then update the Julia compat?

David Widmann: Well, it's not only gamma_inc but similar problems exist for beta_inc and beta_inc_inv. And fixing the function calls is by far not sufficient to fix the test errors on Julia < 1.3 - one runs into all kinds of bugs in SpecialFunctions that are only fixed in more recent versions that don't support Julia < 1.3. And it's impossible/undesirable to fix all these issues in StatsFuns for older versions I think. So we really really have to correct the SpecialFunctions compat entry, implying that we also have to fix the Julia compat entry.

@devmotion
Copy link
Member

Conclusion: There are too many other issues that are not possible to fix in StatsFuns (e.g., undefined variables in gamma_inc_inv which would require to reimplement the whole function). We will revert the Rmath PR, fix errors on Julia 1.0, and release the Rmath PR as part of StatsFuns 0.10.

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 a pull request may close this issue.

3 participants