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

Improved the precision (and probably speed) of cbrtf. #509

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

Conversation

ZERICO2005
Copy link
Contributor

@ZERICO2005 ZERICO2005 commented Oct 10, 2024

Minimum precision: 21.415037 bits at both +4.251052856e+02 and +7.969426579e+17 for cbrtf.

Precision is calculated as log2(fabs( true_cbrt(x) - approx_cbrt(x) )) - ilogb(fabs( true_cbrt(x) ))

The previous method of powf(x, 1.0f / 3.0f) had a minimum precision of 21.0 bits at +4.037431946e+02, and a minimum precision 19.299560 bits at +1.187444200e+07, slowly losing precision at larger magnitudes.

Precision tested with 32bit floats on x86_64

src/libc/cbrt.c Outdated Show resolved Hide resolved
@calc84maniac
Copy link
Contributor

I confirmed using the ez80sf tester that this does increase the number of passed tests to about 42% compared to the old function's 11% (when given non-negative finite inputs, since that's all the old function supported). It's not perfect, but it only claims to be 21.0 bits minimum precision so that's to be expected. I did notice for some huge inputs it returns NaN, though.

@mateoconlechuga
Copy link
Member

powf(x, 1/3.f); is smaller tho

@calc84maniac
Copy link
Contributor

powf(x, 1/3.f); is smaller tho

It's also semantically different (especially for negative inputs), so if someone wants to save space (assuming they're already using powf elsewhere) they can call powf like you just specified.

@ZERICO2005
Copy link
Contributor Author

powf(x, 1/3.f); is smaller tho

True, although it should probably be changed to this

if ( x == 0.0f || !isfinite(x) ) {
	return x;
}
return copysignf(powf(fabsf(x), 1/3.f), x);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants