Bessels.jl icon indicating copy to clipboard operation
Bessels.jl copied to clipboard

Scope of Bessels.jl and inclusion of other special functions (e.g. gamma, airy, scorer)

Open heltonmc opened this issue 2 years ago • 26 comments

EDIT: This issue started with discussion on the gamma implementation but has moved to discussing the scope of this package and inclusion of other special functions or splitting into smaller packages. Please leave comments on what functions you would like to be included here or if you would like smaller packages.

SPLIT FROM https://github.com/heltonmc/Bessels.jl/pull/26#issuecomment-1200481143

A small note: I was perusing your gamma implementation and I think there are a couple small improvements that could be made, assuming they don't cost precision somehow. For one, if I change the v = x^(0.5*x - 0.25) to v = exp((0.5*x-0.25)*log(x)) in large_gamma, I see a big speedup. And it doesn't seem like the log would make an issue, because you'll never hit that branch when x is zero.

Second, if there's a reason not to do that, then an intermediate method like this seems to be faster when x > 11.5 but it isn't too too large:

function smallish_gamma(x)
  _x = min(x, x-ceil(x-11.5))
  _g = small_gamma(_x)
  while _x < x # use the fact that gamma(x+1) == x*gamma(x)
    _g *= _x
    _x += one(_x)
  end
  _g
end

You could then branch on x < 30.0 or something instead and at least hit the large_gamma branch less often.

I haven't checked very hard to see if these tricks introduce new issues, but I wouldn't think that they do.

Originally posted by @cgeoga in https://github.com/heltonmc/Bessels.jl/issues/26#issuecomment-1200481143

heltonmc avatar Aug 01 '22 00:08 heltonmc

Thank you for looking at this @cgeoga !!! Let's discuss here to try and keep the other thread focused on bessely.

I'm very happy to take PRs on this in general which we can iron out potentially a better implementation of this. As mentioned https://github.com/heltonmc/Bessels.jl/pull/26#issuecomment-1200514696 we would definitely need to test accuracy for this. The good thing about 1d functions is we can exhaustively test all the ranges. I would say for a function as important as gamma here keeping the max error below ~5 ULP would be ideal. I need to move the gamma function into its own file as well to keep it separate and add tests as well. But as mentioned hopefully the powers are faster in 1.9, though I just did a quick test on a very old 1.9 branch and found it was much slower than 1.7, so keeping track of this by julia version would also be very helpful.

heltonmc avatar Aug 01 '22 00:08 heltonmc

On a quick first test it looks like the first suggestion increased the mean ULP from 0.26595 to 1.98555 and the maximum ULP from 3 to 571.0 at x=162.81202133604663.

heltonmc avatar Aug 01 '22 00:08 heltonmc

One thing worth noting is that although these techniques won't work for Float64, for Float32, there are a lot of options opened since you can do the internals in Float64 to avoid loosing precision.

oscardssmith avatar Aug 01 '22 00:08 oscardssmith

Makes sense about moving the issues and apologies for the clutter. Also, wow about the additional error. I assumed it would be a little bit worse, but that is certainly more than I expected.

You've mentioned these back-of-the-envelope errors in terms of ULPs before---is there some easy demo you can point me to so I can figure out how to do that kind of error testing myself? I should learn how to do this anyway, and then I can sort of pre-screen some of these ideas before posting.

cgeoga avatar Aug 01 '22 01:08 cgeoga

I haven't tried it out but see Oscar's https://github.com/JuliaMath/FunctionAccuracyTests.jl. Though in this case I just tested against SpecialFunctions.jl

import SpecialFunctions
julia> x = rand(10000)*170
julia> maximum(abs.(1 .- SpecialFunctions.gamma.(BigFloat.(x)) ./ gamma.(x)))/ eps(Float64)

heltonmc avatar Aug 01 '22 01:08 heltonmc

If you want more details on my thought process here, my Juliacon talk was on floating point math implementation and testing. https://www.youtube.com/watch?v=-tFS4PUHqT0

oscardssmith avatar Aug 01 '22 01:08 oscardssmith

Nice, thank you both! Will do some studying.

cgeoga avatar Aug 01 '22 01:08 cgeoga

Can you guys comment a bit more on the current gamma implementation defined in Bessels.jl? Is it ready to use? Why is it not exported? I have a package that depends on both besselk and gamma from SpecialFunctions.jl and would like to replace this dependency by Bessels.jl that is pure Julia with zero dependencies. However, I would like to understand the future plans better before making this migration.

juliohm avatar Sep 09 '22 19:09 juliohm

The function is definitely ready to use. We use it in practically every function in this library which is why we rolled our own implementation that is much faster. It has a max ULP around 2.5-3. But it does have some limitations as it only works for real arguments and for single and double precision. So if you need it for complex or higher precision it won't work.

The reason it is not exported is because this is probably not the best package for a native gamma function implementation. There have been some discussion about moving it over to SpecialFunctions.jl but my plan was to actually create a lightweight Gamma.jl package that just has julia implementations of gamma and similar functions. So I'd be hesitant to export it here as it is likely that we will move it out of the package in the future.

heltonmc avatar Sep 09 '22 19:09 heltonmc

+1 for a Gamma.jl package, that would be great.

cgeoga avatar Sep 09 '22 19:09 cgeoga

Awesome @heltonmc , thank you for the prompt reply. I am also +1 for Gamma.jl or maybe a new package that wraps all these special functions in pure Julia, maybe SpecialFuns.jl?

juliohm avatar Sep 09 '22 19:09 juliohm

the argument for splitting them up is that a lot of libraries want to use gamma functions without the precompilation time of loading everything.

oscardssmith avatar Sep 09 '22 19:09 oscardssmith

Thank you @oscardssmith for adding the comment. Is the compilation time a real issue for Bessels.jl or is this another case of premature modularization? I feel that many of these package splits in Julia are more harmful than helpful, specially when it comes to maintenance. The collateral effect of having many packages is that some of them die after a while without maintainers. Working on the same repo with the same code style, multiple maintainers, etc. pays off in my opinion.

juliohm avatar Sep 09 '22 20:09 juliohm

Well using SpecialFunctions is already .35 seconds (and most of the code is just calling C functions).

oscardssmith avatar Sep 09 '22 20:09 oscardssmith

@juliohm that was my general idea to have a higher level package that wraps all of the mathematical functions outside of Base. I'd like to see more of a canonicalization of the math functions in Julia (particularly the special functions). In my opinion there are many packages that do similar things but the quality is variable and sometimes hard to discern for new users. And they are often put together quickly for a specific application, registered, and then never maintained after…. A python user is immediately going to SciPy or mpmath and they have some sense of quality and stability which I think is a good thing.

Part of the problem with SpecialFunctions.jl is it is actually hard to find maintainers and get things merged where the scope is not so clearly defined (and maybe nobody feels any kind of authority?). One example that I think is concerning is the most recent PR and issue about the fresnel integrals which existed in a different package where a first time contributor to the language submitted a PR to add that functionality that was carefully reviewed by a primary contributor to the language but was decided that it is better in a separate package. So that contributor submitted to the other package but that package is unmaintained…. now we are point people on SpecialFuncitons.jl to an unmaintained package where the existing code base can be improved. So to your point that is definitely an issue with all of these smaller packages. Though, ideally these can be brought under a higher level package under an organization so we can continue to improve them.

So yes… what to do with gamma 🤷‍♂️ ? I’m happy for this package to have special functions but I start to become a bit worried about scope. Which is something I think SpecialFunctions.jl is not so clearly defined. Should we also include loggamma, beta, polygama, digamma, and their incomplete forms here? But I feel like then why not just start making Bessels.jl (or rename it) to just general special functions. I still think the right thing to do is try to improve the scattered special functions already registered in the general repository and improve them and then bring them under some higher organization and repository to standardize in some way. For example, I’ve been spending some time trying to improve Struve.jl so that hopefully we can have several similar quality libraries to piece together.

But I think I agree with your overall sentiment…. Though maybe I was hoping to improve the already existing registered with very defined scope that a higher level package can reexport. So new users can be a little more confident in the quality of the package they are using and having an easy interface with a single package. Though say if you wanted just the gamma function, you can then just depend on that package which has a very defined scope and very lightweight.

Really appreciate your thoughts and definitely welcome to other suggestions!

heltonmc avatar Sep 10 '22 22:09 heltonmc

Thank you all for the very insightful comments. You certainly have better sense of scope and can decide on a good tradeoff for these projects. I just commented as an end user who has a very superficial view of the functionality and would pretty much enjoy a single dependency a la SpecialFuns.jl in all packages I maintain. If you feel that users will benefit from tiny packages with just one math function, please go ahead. My impression is that most people using Bessel will also need Gamma and related functionality.

Em sáb., 10 de set. de 2022 19:42, Michael Helton @.***> escreveu:

@juliohm https://github.com/juliohm that was my general idea to have a higher level package that wraps all of the mathematical functions outside of Base. I'd like to see more of a canonicalization of the math functions in Julia (particularly the special functions). In my opinion there are many packages that do similar things but the quality is variable and sometimes hard to discern for new users. And they are often put together quickly for a specific application, registered, and then never maintained after…. A python user is immediately going to SciPy or mpmath and they have some sense of quality and stability which I think is a good thing.

Part of the problem with SpecialFunctions.jl is it is actually hard to find maintainers and get things merged where the scope is not so clearly defined (and maybe nobody feels any kind of authority?). One example that I think is concerning is the most recent PR and issue about the fresnel integrals which existed in a different package where a first time contributor to the language submitted a PR to add that functionality that was carefully reviewed by a primary contributor to the language but was decided that it is better in a separate package. So that contributor submitted to the other package but that package is unmaintained…. now we are point people on SpecialFuncitons.jl to an unmaintained package where the existing code base can be improved. So to your point that is definitely an issue with all of these smaller packages. Though, ideally these can be brought under a higher level package under an organization so we can continue to improve them.

So yes… what to do with gamma 🤷‍♂️ ? I’m happy for this package to have special functions but I start to become a bit worried about scope. Which is something I think SpecialFunctions.jl is not so clearly defined. Should we also include loggamma, beta, polygama, digamma, and their incomplete forms here? But I feel like then why not just start making Bessels.jl (or rename it) to just general special functions. I still think the right thing to do is try to improve the scattered special functions already registered in the general repository and improve them and then bring them under some higher organization and repository to standardize in some way. For example, I’ve been spending some time trying to improve Struve.jl https://github.com/gwater/Struve.jl so that hopefully we can have several similar quality libraries to piece together.

But I think I agree with your overall sentiment…. Though maybe I was hoping to improve the already existing registered with very defined scope that a higher level package can reexport. So new users can be a little more confident in the quality of the package they are using and having an easy interface with a single package. Though say if you wanted just the gamma function, you can then just depend on that package which has a very defined scope and very lightweight.

Really appreciate your thoughts and definitely welcome to other suggestions!

— Reply to this email directly, view it on GitHub https://github.com/JuliaMath/Bessels.jl/issues/27#issuecomment-1242815205, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3PSITQYINPLBQG4MELV5UFGZANCNFSM55FYNCXQ . You are receiving this because you were mentioned.Message ID: @.***>

juliohm avatar Sep 10 '22 22:09 juliohm

Yes - and that is definitely the most important thing to consider. We should probably be a bit more mindful on the best thing for users here. It's probably best to only have one dependency in their tree so that it is easier to manage versioning etc.

But I don't think it's wise to have tiny packages like you mentioned. I'd like to have packages that have closely related families of functions. For example, here I plan to give bessel functions, modified bessel functions, spherical bessel functions, airy functions, and hopefully soon kelvin functions. The amount of source code for all of that is getting quite large and the amount of time it takes to test each function is also increasing. Then a gamma package can contain all the gamma and related functions I mentioned earlier. I'm actually trying to do some convincing over at Struve.jl to include related functions as well but we'll see...

But getting back to your original question. The gamma function here is quite good but will probably be moved to a Gamma.jl package. Then can create a top level package that re-exports all of the special functions.

Name suggestions for a top level package are most welcome! As well as anyone who would like to help with that!

heltonmc avatar Sep 10 '22 23:09 heltonmc

As a side comment, VSCode recently released a nice feature for testing individual test sets, which is wonderful: https://discourse.julialang.org/t/prerelease-of-new-testing-framework-and-test-run-ui-in-vs-code/86355

So long testing times may not be an issue in the near future.

Em sáb., 10 de set. de 2022 20:18, Michael Helton @.***> escreveu:

Yes - and that is definitely the most important thing to consider. We should probably be a bit more mindful on the best thing for users here. It's probably best to only have one dependency in their tree so that it is easier to manage versioning etc.

But I don't think it's wise to have tiny packages like you mentioned. I'd like to have packages that have closely related families of functions. For example, here I plan to give bessel functions, modified bessel functions, spherical bessel functions, airy functions, and hopefully soon kelvin functions. The amount of source code for all of that is getting quite large and the amount of time it takes to test each function is also increasing. Then a gamma package can contain all the gamma and related functions I mentioned earlier. I'm actually trying to do some convincing over at Struve.jl to include related functions as well but we'll see...

But getting back to your original question. The gamma function here is quite good but will probably be moved to a Gamma.jl package. Then can create a top level package that re-exports all of the special functions.

Name suggestions for a top level package are most welcome! As well as anyone who would like to help with that!

— Reply to this email directly, view it on GitHub https://github.com/JuliaMath/Bessels.jl/issues/27#issuecomment-1242818484, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3JD6MTDG3EBQPTLM6DV5UJLLANCNFSM55FYNCXQ . You are receiving this because you were mentioned.Message ID: @.***>

juliohm avatar Sep 10 '22 23:09 juliohm

O wow thank you!! I've been locally commenting out test groups not to run which is definitely a bit of a hack.

heltonmc avatar Sep 10 '22 23:09 heltonmc

@heltonmc any timeline for the Gamma.jl package? I am looking forward to drop SpecialFunctions.jl from my dependencies and favor Bessels.jl + Gamma.jl. In the meantime, can I safely depend on Besses.jl and use Bessels.gamma explicitly even if it is not part of the public API? Are you planning to release a minor release when it is moved to Gamma.jl?

juliohm avatar Sep 12 '22 21:09 juliohm

You can explicitly depend on Bessels.gamma yes (it's an explicitly tested function anyway). I will definitely follow the guidelines I lay out at https://github.com/JuliaMath/Bessels.jl/blob/master/NEWS.md#changelog on when to increment minor versions and include Bessels.gamma as an unoffical exported function. When we move gamma out of this package we will increment the minor version (e.g., v0.2 -> v0.3) and make very big mention of it in the NEWS.md file. I'll comment here as well.

The timeline is tricky as I want to be as fair as possible giving credit to all the original authors of julia code in SpecialFunctions.jl that worked on gamma like functions (there's a few functions I'd like to move over along with the implementation here). To me the best way to do this is by preserving all of the git history for the necessary files. Of course I am not a maintainer or contributor explicitly at SpecialFunctions.jl so wouldn't want to step on any toes. I'll work to see if I can get this done this week.... but I really don't want to make any promises there.

heltonmc avatar Sep 12 '22 21:09 heltonmc

And just another comment so it's clear for anyone using gamma. If you call gamma with Bessels.gamma even if we move the function to another package that call will still work in your code so you don't really have to worry about any breaking changes (the API for gamma is very stable). Every function in this package relies on gamma so it will also be loaded within the module either by a using Gamma or with the function defined within the package as it is now.

Now is that good practice...... probably not 🤷‍♂️ but this package will always depend heavily on gamma so that functionality will always be there.

heltonmc avatar Sep 13 '22 21:09 heltonmc

I've updated the title of this issue as it has evolved into the scope of this project itself rather than the actual gamma implementation.

I think I've become a bit more persuaded by @juliohm and some others who have reached out to have a more centralized package instead of it being fragmentated. As of now @time_imports using Bessels takes about 10 milliseconds on my computer which I think makes this package even though it has a large amount of code fairly lightweight for users and won't contribute to longer package load times. However, the TTFX will be longer as most of the time is on code generation.

Another thing I'm starting to realize is testing might be better if all related functions with overlapping code were under the same folder that were tested simultaneously by CI. For example, there is sometimes a chain of function calls like gamma->besselk->airy->inhomog. airy. But in some domains, besselk may need to call the airy implementation. So I'm thinking it might make more sense if there is any overlapping code that they share the same project to make sure any changes are rigorously tested within the scope of all dependent special functions. Of course in my case gamma is always independent and could be separate....

Therefore, I might suggest that this package be home to any functions listed in Chapter 9-11 of NIST (https://dlmf.nist.gov/) which is also how mpmath (https://mpmath.org/doc/current/functions/index.html) groups them though they also include Chapter 12 and some of the special cases of hypergeomtric functions.

I think I may make a discourse announcement for this package and maybe some discussion can continue there...

Thanks everyone for the thoughts!

heltonmc avatar Sep 28 '22 13:09 heltonmc

Having a single umbrella for these functions will save you a lot of maintenance headache for sure @heltonmc. Looking forward to seeing it become the next generation special functions in Julia 🚀

juliohm avatar Sep 28 '22 14:09 juliohm

@heltonmc I am trying to purge SpecialFunctions.jl from my indirect dependencies...

Do we have the functionality to replace these three functions from this core package already?

https://github.com/JuliaGraphics/ColorVectorSpace.jl/blob/8ec1d3549cab5064be2f09b8bb8de6f502c9c2ba/src/ColorVectorSpace.jl#L26

I can submit a PR to ColorVectorSpace.jl to remove SpecialFunctions.jl and add Bessels.jl instead.

juliohm avatar Sep 30 '22 17:09 juliohm

I actually thought SpecialFunctions already had a loggamma implementation but it looks like it’s just for complex values. I am on vacation until next week so can’t tell exactly what is going on. I think having a good loggamma implementation here is important as it is needed. And I believe logabsgamma can be called from that.

Is ‘lfact’ deprecated? Is that just the logarithm of the factorial? That could also be easily defined from the loggamma algorithm… seems like an important function. I actually had an implementation of loggamma early on that I’ll try to track down again. I’ll try to look into that next week.

But to answer your question only gamma is supported right now and the other ones should really be defined from a dedicated loggamma implementation.

heltonmc avatar Sep 30 '22 18:09 heltonmc