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 log trick to roots_jacobi and roots_sh_jacobi #12580

Closed
wants to merge 0 commits into from

Conversation

itamarfaran
Copy link

Reference issue

closes #12425: roots_sh_jacobi: Error handling for OverflowError

What does this implement/fix?

add option that allows returning of the log values in orthogonal._gen_roots_and_weights when symmetrize=False

Additional information

needed to imitate statmod::gauss.quad.prob function in R

Copy link
Contributor

@sethtroisi sethtroisi left a comment

Choose a reason for hiding this comment

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

I failing tests are

FAILED scipy/special/tests/test_orthogonal.py::test_roots_jacobi - AssertionE...
FAILED scipy/special/tests/test_orthogonal.py::test_roots_sh_jacobi - Asserti...

So that needs to be looked into

scipy/special/orthogonal.py Outdated Show resolved Hide resolved
scipy/special/orthogonal.py Outdated Show resolved Hide resolved
@itamarfaran
Copy link
Author

itamarfaran commented Apr 5, 2021

@sethtroisi the tests pass now for log_weights=False. however, when testing for log_weights=True, I'm getting the following failure in test_orthogonal.py#L331:

I'm getting an absolute difference of .023, which is a relative difference of about 1.69e-15 (mu is in the scale of 10^13). So the test is a bit to harsh for taking exponents and logs. what should I do about that?

@itamarfaran
Copy link
Author

@person142 any new with this pr :)?

@person142
Copy link
Member

Ach let me try to find some time to review... first thought though is that a flag determining whether we return weights or log weights is non-ideal API-wise; generally outputs of functions shouldn't change completely depending on flags. (Admittedly yes the API already does that, but we can no longer correct that because of back compat. Would be nice to not make the situation worse though.)

@itamarfaran
Copy link
Author

@person142 would it be better to open a completely new module with 'roots_log_ ...' functions instead?

@person142
Copy link
Member

would it be better to open a completely new module with 'roots_log_ ...' functions instead?

Yes, new functions would be the way to go I think. It would match well with the rest of the API-e.g. loggamma versus gamma, log_ndtr versus ndtr and so on. But before making you put more effort into this, let me read #12425 more carefully first... might not have time for that until next week though.

@lucascolley lucascolley added enhancement A new feature or improvement needs-decision Items that need further discussion before they are merged or closed labels Dec 23, 2023
@lucascolley lucascolley changed the title add log trick to roots_jacobi an roots_jacobi_sh ENH: special: add log trick to roots_jacobi and roots_sh_jacobi Mar 14, 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-decision Items that need further discussion before they are merged or closed scipy.special
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roots_sh_jacobi: Error handling for OverflowError
5 participants