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

Allow 1 ULP of error in exp, ln, log10 and power GDA tests #152

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

Conversation

rdingwall
Copy link
Collaborator

@rdingwall rdingwall commented May 23, 2020

In the GDA arithmetic specification, inexact exp, ln, log10 and power operations may be up to 1 ulp (unit in last place) in error: http://speleotrove.com/decimal/daops.html

In the GDA arithmetic specification, inexact exp, ln, log10 and power
operations may be up to 1 ulp (unit in last place) in error.
@rdingwall rdingwall force-pushed the gda_dectest_allow_1ulp_error branch from e486b12 to e65a659 Compare May 24, 2020 00:30
@rdingwall rdingwall requested a review from ericlagergren May 25, 2020 12:37
if isInexact(z) && allow1ULPError[c.Op] && equalWithin1ULP(z, r) {
s = append(s, "passed within 1 ULP")
}
t.Log(strings.Join(s, "\n"), "\n")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I refactored this string builder from one fmt.Sprintf() into strings.Join() to preserve test output (but have the ability to insert the optional ULP line in the middle)

if !x.IsFinite() || !y.IsFinite() {
return false
}
formX, negX, coeffX, expX := x.Decompose(make([]byte, 2<<3))
Copy link
Owner

Choose a reason for hiding this comment

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

Off hand (I haven’t tested it yet, or had my coffee :-), I’m not sure if this is accurate. [123, -2] and [1234, -3] should compare equal within 1 ULP, but would fail here since their exponents differ.

Copy link
Owner

@ericlagergren ericlagergren Aug 7, 2021

Choose a reason for hiding this comment

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

Hey @rdingwall, apparently I had added this comment (one year ago!) but never actually clicked “submit” on the review. Sorry for that. I only noticed when I got the email that you’d rebased!

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