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

add logabstanh #86

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

oscardssmith
Copy link
Contributor

as requested by @c05 on slack. This has 15 ULP for Float64, and 5 ULP for Float32.

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

Thank you! Can you add tests and update the docs? Probably should also add a ChainRules definition.

src/basicfuns.jl Outdated Show resolved Hide resolved
src/basicfuns.jl Outdated Show resolved Hide resolved
src/basicfuns.jl Outdated Show resolved Hide resolved
src/basicfuns.jl Outdated Show resolved Hide resolved
src/basicfuns.jl Outdated
The implementation ensures `logabstanh(-x) = logabstanh(x)`.
"""
function logabstanh(x::Real)
return log1p(-2/((exp(2abs(x))+1)))
Copy link
Member

Choose a reason for hiding this comment

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

As indicated by the special cases close to 0 for Float32 and Float64, maybe a safer default would be

Suggested change
return log1p(-2/((exp(2abs(x))+1)))
return log(abs(tanh(x)))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the version I wrote stays more accurate overall but I'll do a test

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's possible to do such an analysis? You can't realistically test any subtype of Real.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for any AbstractFloat log(tanh(x)) will be inaccurate for large x since tanh(x) will be very close to 1.

oscardssmith and others added 4 commits November 16, 2024 09:36
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
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.

2 participants