Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0002688 [Squeak] Kernel minor always 02-08-06 02:06 10-21-06 04:27
Reporter nicolas cellier View Status public  
Assigned To
Priority normal Resolution open  
Status new   Product Version 3.8
Summary 0002688: aNumber = (aNumber + 0 i) answer false
Description self
  should: 1 = (1 + 0 i)
  description: 'due to a premature
       isNumber ifFalse: [^false].
    this message answer false'.

self should: ((1+0i)=1) = (1=(1+0i)
  description: 'equality test should be commutative'.
Additional Information aComplex cannot answer true to isNumber because it is not kindOf: Number.

isNumber test should be replaced with isArithmeticValue, and Complex Quaternion or whatever RealNumber extension should answer true to this one.

Note that VW has ArithmeticValue as superclass of both Complex and Number.
Attached Files  ComplexEqualTest.3.cs [^] (763 bytes) 02-08-06 22:29
 ComplexEqualPatch.1.cs [^] (2,494 bytes) 02-08-06 22:29

- Relationships

SYSTEM WARNING: Creating default object from empty value

SYSTEM WARNING: Creating default object from empty value

SYSTEM WARNING: Creating default object from empty value

related to 0002689new  isComplex method name is bad 
related to 0003725closed  [FIX] ComplexTest>>testEquality 
related to 0004262closed KenCausey SUnit test ComplexTest>>testEquality fails for an collection 
related to 0004261closed  SUnit test ComplexTest>>testEquality fails for nil 
related to 0003311new  Complex Numbers are Wacky 

- Notes
(0003726 - 381 - 435 - 435 - 435 - 435 - 435)
nicolas cellier
02-08-06 22:34

I propose a test and a patch.

implement isNumberExtension in Object Complex Number (and future Quaternion Octonion etc... if any).

in = message, test isNumberExtension instead of isNumber.
if false, answer false.
if true, then proceed with current code (will fallback in adaptTo... and succeed).

Rationale:
Complex are Number extension, they can be equal to a number...
 
(0003749 - 17 - 17 - 17 - 17 - 17 - 17)
MarcusDenker
02-11-06 19:24

added test to 3.9
 
(0004576 - 124 - 148 - 148 - 148 - 148 - 148)
nicolas cellier
03-26-06 21:42

Just to remind that proposed patch also cover this bug:

1 i = nil

(see '[BUG] Complex equality problem' on squeak-dev)
 
(0004891 - 71 - 71 - 197 - 197 - 197 - 197)
nicolas cellier
05-08-06 10:33

Another solution is discussed at http://bugs.impara.de/view.php?id=3540 [^]
 
(0007728 - 142 - 160 - 288 - 288 - 288 - 288)
KenD
10-19-06 06:37

Note that the alternate implementation available via 0003311 (http://bugs.impara.de/view.php?id=3311) [^] does NOT have this bug.

$0.02,
-KenD
 
(0007737 - 755 - 803 - 803 - 803 - 803 - 803)
nicolas cellier
10-19-06 21:04

Hi Ken.

If you define (Complex>>isNumber ^true), then you suppress the bug, but you might also introduce new bugs for applications expecting only real numbers and testing that using message isNumber...

This is for thess backward compatibility reasons that i introduced a new message.
Alternatively, it would be necessary to add a isRealNumber message to Number, then replace some of the isNumber senders with isRealNumber...

Another reason is that ComplexNumber are not the only Number extensions as shown with my Quaternion example. In order to avoid same bug (1+0i+0j+0k)=0, Quaternion isNumber should also answer true, something that we could agree on, but which sounds less obvious than ComplexNumber (We do not name it QuaternionNumber...).
 
(0007739 - 381 - 427 - 427 - 427 - 427 - 427)
KenD
10-20-06 03:45

Hi nicolas,

Exact reals might be implemented as continued fractions, linear fractional transforms... Reals might use interval math, ...

I don't see how having complex numbers not be numbers fixes the case where users of reals numbers expect a floating point representation.

There may be other bugs introduced, but I don't buy the "keep backward compatible bugs" argument.
 
(0007742 - 2722 - 3153 - 3153 - 3153 - 3153 - 3153)
nicolas cellier
10-20-06 17:05
edited on: 10-20-06 17:18

Hi again,

1) exact real numbers are out of scope
2) i do not want the bug to survive, but proposed a more local cosmetic change than yours (message isNumberExtension).

Main advantage of my own patch is that it is cosmetic: it has very few chance of breaking current images and applications.
But on longer term, for sure i adhere to the deeper refactorings you are claiming.

The facts:
- isNumber is so far implemented and interpreted as a short cut for (isKindOf: Number).
- Complex is so far not a subclass of Number.

So isNumber is now synonym to (belong to R), and you propose a semantic change (belong to C).

This potentially is dangerous for existing applications. What would you buy for 2 imaginary euros on your bank account?
This is why if we adopt your semantic, we would need a different message telling that imaginary part is null (my isRealNumber proposal which would be true also for Integer and Fraction)

Other facts:
- (2 imaginaryPart) and (-1 sqrt) are not understood as Complex extensions
- instead, we have to explicitely create a Complex to obtain this behavior
- For this reason, complex with null imaginary part are not converted automatically to real Number part... (as would a Fraction with denominator 1 being converted to Integer).

If we want to change these behaviours, we should think of all implications, and refactorings it will imply.

I am not sure Complex should be a subclass of Number, considering some details like order relationship #< properties.
Another possibility would be the reverse class hierarchy. Every number is a complex number (eventually with null imaginary part)...

(Abstract)ComplexNumber
  (Abstract)(Real)Number
    Integer
    Fraction
    ExactRealNumber
      AlgebraicNumber
      ...
  ImaginaryNumber (Complex concrete class)

Maybe AlgebraicNumber should be automatically converted to Fraction when they are rational...
Where to put Float in this hierarchy is not that obvious... Float representation is just a special case of Rational... Except that operations are inexact.
FixedPoint are pure rational with different Display properties...

If we want to introduce Quaternion in such a scheme, we put AbstractQuaternion above AbstractComplexNumber and concrete Quaternion aside AbstractComplexNumber... Something powerfull, but not really lightweight...

Another possibility is to use Traits instead of inheritance...

So my point of view is that just changing isNumber semantic has not enough benefits by itself in order to justify backward compatibility problems it would introduce. Complex maybe deserve deeper changes so as to have a consistant model.

Finally should Quaternion isNumber return true?

 
(0007743 - 620 - 1623 - 1623 - 1623 - 1623 - 1623)
KenD
10-21-06 04:27
edited on: 10-21-06 04:34

Ah, I am coming from a background of dynamic languages (Scheme, Lisp, Dylan) where: integer C rational C real C complex.

I am surprised to find someone that thinks differently.

E.g. in the Dylan class hierarchy:

               Object
                  |
               Number
                  |
               Complex
                  |
                Real
                ../..\
             Float..Rational
             ../..........\
      -------------....Integer
    ../......|......\
  single-..double-..extended-
    float.....float.....float

What do non-squeak Smalltalks do? ANSI?

 

- Issue History
Date Modified Username Field Change
02-08-06 02:06 nicolas cellier New Issue
02-08-06 02:06 nicolas cellier Status new => assigned
02-08-06 02:06 nicolas cellier Assigned To  => KenCausey
02-08-06 19:07 MarcusDenker Description Updated
02-08-06 19:07 MarcusDenker Assigned To KenCausey =>
02-08-06 19:07 MarcusDenker Status assigned => new
02-08-06 19:07 MarcusDenker Category Any => Kernel
02-08-06 22:29 nicolas cellier File Added: ComplexEqualTest.3.cs
02-08-06 22:29 nicolas cellier File Added: ComplexEqualPatch.1.cs
02-08-06 22:34 nicolas cellier Note Added: 0003726
02-11-06 19:24 MarcusDenker Note Added: 0003749
03-26-06 21:42 nicolas cellier Note Added: 0004576
05-08-06 10:33 nicolas cellier Note Added: 0004891
07-15-06 11:53 MarcusDenker Relationship added related to 0003725
07-15-06 11:54 MarcusDenker Relationship added related to 0004262
07-15-06 11:56 MarcusDenker Relationship added related to 0004261
07-15-06 11:57 MarcusDenker Relationship added related to 0003311
07-22-06 22:20 MarcusDenker Relationship added related to 0002689
10-19-06 06:37 KenD Note Added: 0007728
10-19-06 21:04 nicolas cellier Note Added: 0007737
10-20-06 03:45 KenD Note Added: 0007739
10-20-06 17:05 nicolas cellier Note Added: 0007742
10-20-06 17:18 nicolas cellier Note Edited: 0007742
10-21-06 04:27 KenD Note Added: 0007743
10-21-06 04:30 KenD Note Edited: 0007743
10-21-06 04:32 KenD Note Edited: 0007743
10-21-06 04:33 KenD Note Edited: 0007743
10-21-06 04:34 KenD Note Edited: 0007743


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