Skip to content

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