Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] 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  
Status closed   Product Version 3.9
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...
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