Skip to content

Conversation

ear7h
Copy link
Contributor

@ear7h ear7h commented Sep 26, 2019

  • comparisons with zero are absoloute (epsilon method)
  • comparisons with non-zeros are relative (bit pattern distance method)

I also renamed TOLERANCE to conform with Go conventions

performance difference (slower as expected)

name        old time/op    new time/op    delta
CmpFloat-4    4.89ms ± 1%    5.53ms ± 1%  +12.93%  (p=0.000 n=8+7)

name        old alloc/op   new alloc/op   delta
CmpFloat-4     66.7B ± 1%     75.1B ± 1%  +12.63%  (p=0.001 n=7+7)

name        old allocs/op  new allocs/op  delta
CmpFloat-4      0.00           0.00          ~     (all equal)

* comparisons with zero are absoloute (epsilon method)
* comparisons with non-zeros are relative (bit pattern distance method)
Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

just make that one change, and then this is good to go.

LGTM

cmp/cmp.go Outdated
if d < 0 {
return d > -bitTolerance
} else {
return d < bitTolerance
Copy link
Member

Choose a reason for hiding this comment

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

Drop the else branch, it's unnecessary and can cause a bit of confusion when quickly reading the code.

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

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

LGTM

@gdey gdey merged commit 31ae97d into master Sep 26, 2019
@gdey gdey deleted the cmp branch November 5, 2019 02:07
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