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

ENH: special: add logiv and logive #12641

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sethtroisi
Copy link
Contributor

Fixes #12607

I'm not sure if this is the right structure, the right place, or the right name.

  1. logiv, log_iv, lniv, ivln
    context: scipy.special contains ndtr(x) and log_ndtr(x) | gamma(z), gammaln(x), and loggamma(z) | beta(a, b), betaln(a, b)

  2. I'm not sure what precision goal I should aim for;

    1. percent error seem to be very small when v is large but can be as high as 1e-9 when v is just greater than the threshold.
  3. I'm not sure if I need to support complex numbers for these functions.

  4. I don't yet support array/arraylike input.

    1. Is there a guide or example PR someone can point me at?

@sethtroisi
Copy link
Contributor Author

@WarrenWeckesser I'd like to hear your opinion if these are worth including.

Copy link
Member

@ev-br ev-br left a comment

Choose a reason for hiding this comment

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

A couple of drive-by comments, not a definitive review:

  • it'd be nice to deduplicate with _hyp0f1, if possible

  • the test_mpmath machinery is very useful for smoking out corner cases. Would be nice to include this

  • best ensure that what you add here is available from the cython_special interface.

  • these functions should be ufuncs, so cython kernels need to be hooked up into the ufunc-generation machinery

"""
Log of modified Bessel function of the first kind of real order.

See `iv` for Parameters and return
Copy link
Member

Choose a reason for hiding this comment

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

Provably want to add a See Also section


__all__ = ['logiv', 'logive']

def logiv(v, z):
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move the dispatch to Cython, so that cython_special interface is consistent.

@sethtroisi
Copy link
Contributor Author

A couple of drive-by comments, not a definitive review:

@ev-br These are very useful.

  • it'd be nice to deduplicate with _hyp0f1, if possible

I keep this in mind but won't look at it till we're closer to the end.

  • the test_mpmath machinery is very useful for smoking out corner cases. Would be nice to include this

I wasn't aware of this test; I added a test_logiv method which confirms accurate to 3e-14

  • best ensure that what you add here is available from the cython_special interface.
  • these functions should be ufuncs, so cython kernels need to be hooked up into the ufunc-generation machinery

Can you point me at some links for doing this? I'm trying to follow 4e86352 but I'm still missing something.

@ev-br
Copy link
Member

ev-br commented Aug 6, 2020

I keep this in mind but won't look at it till we're closer to the end.

Makes sense!

Re ufuncs: you'll need to add an entry to functions.json, the docstrings go to add_newdocs.py.

I'm not sure about the moving parts now: It used to be necessary to run generate_ufuncs.py manually, not sure if it's still needed (if it is, please make a separate GEN commit).

The most authoritative review should be from Josh or Pauli.

@sethtroisi
Copy link
Contributor Author

sethtroisi commented Aug 6, 2020

Re ufuncs: you'll need to add an entry to functions.json, the docstrings go to add_newdocs.py.

I'm not sure about the moving parts now: It used to be necessary to run generate_ufuncs.py manually, not sure if it's still needed (if it is, please make a separate GEN commit).

I don't believe this is needed anymore; _generate_pyx.py seems to be run during setup.py

I defined my function in _logiv.pxd as cdef double log_iv_asym(double v, double z) nogil: and added that definition (dd->d) to functions.json. I can import my function as from ._ufuncs import _log_iv_asym but I'm not able to pass an ndarray as parameter. I suspect and I'm missing a ufuncs wrapper function somewhere (_ufuncs.pyi maybe?)

@ev-br
Copy link
Member

ev-br commented Aug 6, 2020

Seems to work?

In [8]: from scipy.special._ufuncs import _log_iv_asym                          

In [9]: _log_iv_asym([1, 2, 3], [2, 3, 5])                                      
Out[9]: array([0.46121925, 0.80862499, 2.33515425])

However, I'm not sure what is the end goal here: is it really useful to add the asymptotic expansion ufunc to the API (albeit a private part of it). I suspect you'll change it later so that from scipy.special import log_ive works without a python wrapper.

@sethtroisi sethtroisi force-pushed the enh_log_ive branch 2 times, most recently from 9463bcf to f368804 Compare August 6, 2020 21:07
@j-bowhay j-bowhay added enhancement A new feature or improvement needs-work Items that are pending response from the author labels Nov 6, 2022
@lucascolley lucascolley marked this pull request as draft March 14, 2024 21:36
@lucascolley lucascolley changed the title [WIP] ENH: Add scipy.special.logiv and logive [WIP] ENH: special: add logiv and logive May 18, 2024
@dschmitz89
Copy link
Contributor

CC @fancidev : these functions would come in very handy for statistical computations, e. g. the Von Mises-Fisher distribution, in case you are interested ;)

@fancidev
Copy link
Contributor

@dschmitz89 The log BesselI function would be very useful. I also wanted it (as well as log BesselK function) when I worked on a compound Poisson process. The ive and kve functions were helpful but they still overflow / underflow for large $x$ if I remember correctly.

However, I guess it’s quite challenging to write a solid logiv. I’ll probably do some research after I finish some of the other easier ones, especially the univariate “all roots” finder which I’m quite interested in.

@lucascolley lucascolley changed the title [WIP] ENH: special: add logiv and logive ENH: special: add logiv and logive Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement needs-work Items that are pending response from the author scipy.special
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding log(ive) Modified Bessel Function of the first kind
5 participants