Anonymous | Login | 01-24-2021 22:49 UTC |
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 |
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() |
||||||||
|
![]() |
|||||||||||
SYSTEM WARNING: Creating default object from empty value
|
![]() |
|
(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 [^] |
Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
124 total queries executed. 60 unique queries executed. |