Mantis Bugtracker
Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Advanced Details [ Jump to Notes ] [ View Simple ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0003374 [Squeak] Kernel minor always 03-28-06 22:36 04-18-10 21:58
Reporter nicolas cellier View Status public  
Assigned To nicolas cellier
Priority normal Resolution fixed Platform
Status closed   OS
Projection none   OS Version
ETA none Fixed in Version trunk Product Version 3.9
  Product Build
Summary 0003374: Arithmetic equality should be transitive
Description Due to coercion of Float/Integer or Float/Fraction using inexact Float arithmetic (converting to Float and thus loosing bits), we can have 2 objects that are different, but answer true when asked if equal to a third Object.

This broken behaviour can introduce nasty bugs in Set for example, depending on the order elements are added.

Example:
  | a b c |
  a := 16rFFFFFFFFFFFFF81.
  b := 16rFFFFFFFFFFFFF82.
  c := a asFloat.
  {a = b.
   a = c.
   b = c.}

a equal c, b equal c, but a does not equal b...
Steps To Reproduce
Additional Information
Right answer should be to coerce to more general class.
And Fraction is more general than Float, because Float is a subset of Fraction.

Thus, we would not loose bits and have correct equality tests.
Attached Files  Float-Compare-Patch.1.cs [^] (8,293 bytes) 03-28-06 23:49
 Float-Compare-Patch.2.cs [^] (8,879 bytes) 03-29-06 01:07
 Float-Compare-Test.1.cs [^] (1,397 bytes) 03-29-06 02:40
 Float-Compare-Patch.3.cs [^] (8,878 bytes) 05-29-08 23:24
 Float-Compare-Test.2.cs [^] (1,516 bytes) 05-29-08 23:25
 M3374-NumberHash-Patch-nice.1.cs [^] (1,706 bytes) 01-04-09 22:05
 M3374-Float-Compare-Patch-nice.1.cs [^] (8,579 bytes) 01-04-09 22:30
 M3374-Float-Compare-Test-nice.1.cs [^] (1,674 bytes) 01-04-09 22:31
 M3374-Float-Compare-Patch-nice.2.cs [^] (8,585 bytes) 07-10-09 20:21
 M3374-Float-Compare-Test-nice.2.cs [^] (2,407 bytes) 07-10-09 20:30

- Relationships

SYSTEM WARNING: Creating default object from empty value

related to 0006601closed nicolas cellier Float hash incorrect ( 2=2.0 , but 2 hash~=2.0 hash) 
related to 0007361closed lewis Float >= primitive failure code sends > instead of >= 

- Notes
(0004595 - 409 - 448 - 448 - 448 - 448 - 448)
nicolas cellier
03-28-06 23:53

 First patch attempt.
I used a new method adaptToFloat:andCompare:
because testing against all comparison selector would otherwise speed down usual arithmetic (* + - /).

Comparison to NaN is broken, because it won't always answer false, but i think this behavior was already there before me.
2 >= NaN is true for example.
(my opinion anyway is that an exception should be raised NaN cannot be compared).
 
(0004599 - 289 - 361 - 487 - 487 - 487 - 487)
nicolas cellier
03-29-06 01:09
edited on: 03-29-06 02:42

 Second version includes new Float>>hash coming with new comparison.

TODO: rehash all the Set/Dictionary/anything else ?
(should be included in the cs)

TODO: speed up horrible Float>>hash slow down.

TODO: merge with ComplexEqualPatch
from http://bugs.impara.de/view.php?id=2688 [^]
 
(0011126 - 516 - 540 - 712 - 712 - 712 - 712)
nicolas cellier
09-13-07 23:48

 Some additionnal wisdom from outside:

http://www.lispworks.com/documentation/lcl50/aug/aug-170.html [^]

In general, when an operation involves both a rational and a floating-point argument, the rational number is first converted to floating-point format, and then the operation is performed. This conversion process is called floating-point contagion. However, for numerical equality comparisons, the arguments are compared using rational arithmetic to ensure transitivity of the equality (or inequality) relation.
 
(0012225 - 617 - 692 - 692 - 692 - 692 - 692)
nicolas cellier
05-29-08 23:24
edited on: 05-29-08 23:42

 I did not test comparison with Float infinity... and a message was sent to self rather than rcvr in adaptToFloat:andCompare:

So i upload an updated test and patch

The rule i have chosen for comparison to infinity is:
every integer is less than Float infinity (and greater than negative infinity), even if it would coerce to Float infinity (resp. negative infinity).

A nasty side effect is that (13 raisedTo: 400) < (11 raisedTo: 400) asFloat.
Since asFloat inexactly coerce a finite number to infinity, such nasty effects cannot be avoided.

Another possible rule would have been to raise an Exception.
 
(0012226 - 153 - 207 - 207 - 207 - 207 - 207)
nicolas cellier
05-29-08 23:29

 "fix begin"
Installer mantis bug: 3374 fix:'Float-Compare-Patch.3.cs'.
"fix test"
Installer mantis bug: 3374 fix:'Float-Compare-Test.2.cs'.
"fix end"
 
(0012892 - 1471 - 1690 - 1690 - 1982 - 1982 - 1982)
nicolas cellier
01-04-09 22:20
edited on: 01-04-09 22:22

 With M3374-NumberHash-Patch-nice.1.cs , I attempt a new patch for hash based on 0006601 and with prerequisite 0006983 for #isFinite selector.

- (Float hash) is coerced to (Integer hash) when fractionPart = 0.0
else, the hash code is modified to use 56 out of 64 bits (instead of 32 out of 64)
- Integer hash is unchanged
- (Fraction hash) is coerced to (Float hash) when denominator is a power of two
otherwise, it cannot be exactly equal to a Float. I assume the Fraction is reduced

Small Float hash speed is a little downgraded, but not that much.
Big Float hash speed is degraded by a factor up to 10, but I claim these Float are rare ( see argument at 0006601 ).
Of course, one can always exhibit an unlucky example, but I prefer a slow down to a false result.
Make it right > make it fast.

I would have liked to use other Float hash algorithms like:

[| f |
f := -1.7014118346046921e38.
ByteArray hashBytes: f startingWith: Float hash] bench.

but it does not work...
And this one is slow:

[| f |
f := -1.7014118346046921e38.
((ByteArray new: 8) doubleAt: 1 put: f bigEndian: true) hash] bench.

This one is a little slower than adopted:
[| f |
f := -1.7014118346046921e38.
((f basicAt: 1) bitShift: -4) + ((f basicAt: 2) bitAnd: 16r0FFFFFFF)] bench.

I would prefer this one if hashMultiply was a fast primitive:
[| f |
f := -1.7014118346046921e38.
(((f basicAt: 1) bitShift: -4) + (f basicAt: 2) hashMultiply) hashMultiply] bench
 
(0012893 - 278 - 344 - 344 - 344 - 344 - 344)
nicolas cellier
01-04-09 22:33

 "fix begin"
Installer mantis ensureFix: 6983.
Installer mantis bug: 3374 fix:'M3374-Float-Compare-Patch-nice.1.cs'.
Installer mantis bug: 3374 fix:'M3374-NumberHash-Patch-nice.1.cs'.
"fix test"
Installer mantis bug: 3374 fix:'M3374-Float-Compare-Test-nice.1.cs'.
"fix end"
 
(0013170 - 56 - 56 - 56 - 169 - 169 - 169)
nicolas cellier
07-10-09 20:32

 The new version I just uploaded is required to fix 0007361
 
(0013171 - 278 - 344 - 344 - 344 - 344 - 344)
nicolas cellier
07-10-09 20:47

 "fix begin"
Installer mantis ensureFix: 6983.
Installer mantis bug: 3374 fix:'M3374-Float-Compare-Patch-nice.2.cs'.
Installer mantis bug: 3374 fix:'M3374-NumberHash-Patch-nice.1.cs'.
"fix test"
Installer mantis bug: 3374 fix:'M3374-Float-Compare-Test-nice.2.cs'.
"fix end"
 
(0013172 - 235 - 247 - 651 - 651 - 651 - 651)
nicolas cellier
07-10-09 21:22

 There is a (very) long but interesting dicussion starting at http://permalink.gmane.org/gmane.comp.lang.smalltalk.pharo.devel/10642 [^]
http://thread.gmane.org/gmane.comp.lang.smalltalk.sapphire.devel/10223/focus=10228 [^]
about this change.
 
(0013510 - 114 - 120 - 430 - 430 - 430 - 430)
nicolas cellier
02-13-10 04:21

 in http://source.squeak.org/trunk/Kernel-nice.397.mcz [^]
and http://source.squeak.org/trunk/KernelTests-nice.134.mcz [^]
 

- Issue History
Date Modified Username Field Change
03-28-06 22:36 nicolas cellier New Issue
03-28-06 23:49 nicolas cellier File Added: Float-Compare-Patch.1.cs
03-28-06 23:53 nicolas cellier Note Added: 0004595
03-29-06 01:07 nicolas cellier File Added: Float-Compare-Patch.2.cs
03-29-06 01:09 nicolas cellier Note Added: 0004599
03-29-06 02:40 nicolas cellier File Added: Float-Compare-Test.1.cs
03-29-06 02:42 nicolas cellier Note Edited: 0004599
09-13-07 23:48 nicolas cellier Note Added: 0011126
09-25-07 01:13 matthewf Relationship added related to 0006601
05-29-08 23:24 nicolas cellier Note Added: 0012225
05-29-08 23:24 nicolas cellier File Added: Float-Compare-Patch.3.cs
05-29-08 23:25 nicolas cellier File Added: Float-Compare-Test.2.cs
05-29-08 23:29 nicolas cellier Note Added: 0012226
05-29-08 23:42 nicolas cellier Note Edited: 0012225
01-04-09 22:05 nicolas cellier File Added: M3374-NumberHash-Patch-nice.1.cs
01-04-09 22:20 nicolas cellier Note Added: 0012892
01-04-09 22:21 nicolas cellier Note Edited: 0012892
01-04-09 22:22 nicolas cellier Note Edited: 0012892
01-04-09 22:30 nicolas cellier File Added: M3374-Float-Compare-Patch-nice.1.cs
01-04-09 22:31 nicolas cellier File Added: M3374-Float-Compare-Test-nice.1.cs
01-04-09 22:33 nicolas cellier Note Added: 0012893
01-09-09 23:59 Keith_Hodges Status new => pending
07-10-09 20:21 nicolas cellier File Added: M3374-Float-Compare-Patch-nice.2.cs
07-10-09 20:30 nicolas cellier File Added: M3374-Float-Compare-Test-nice.2.cs
07-10-09 20:32 nicolas cellier Note Added: 0013170
07-10-09 20:47 nicolas cellier Note Added: 0013171
07-10-09 21:22 nicolas cellier Note Added: 0013172
07-12-09 17:01 KenCausey Relationship added related to 0007361
02-13-10 04:21 nicolas cellier Status pending => resolved
02-13-10 04:21 nicolas cellier Fixed in Version  => trunk
02-13-10 04:21 nicolas cellier Resolution open => fixed
02-13-10 04:21 nicolas cellier Assigned To  => nicolas cellier
02-13-10 04:21 nicolas cellier Note Added: 0013510
04-18-10 21:58 andreas Status resolved => closed

Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
124 total queries executed.
60 unique queries executed.
Powered by Mantis Bugtracker