On Thu, 14 Aug 2008, Randy Portnoff wrote:
Hi Randy,
> I have changed the numeric comparison functions in my Harbour version
> to compensate for floating point issues when comparing numeric values
> (it was must simpler doing it in Harbour than in the PRG code) - I
> round all values to 8 decimal places before making the comparison - I
> have used this for years with many Harbour builds with MSVC v6.
This is technically wrong. IEE758 double value has 53-bit precision
(nearly 16 decimal digit) but the range is much wider 1024-bit (~308
dec digits) and such hack with fixed number of decimal places breaks
very small numbers comparison and has no effect on very big ones.
> I am trying MS Visual Studio 2005 (MSVC v8?) but there seems to be an
> issue with my changes as they relate to hb_numRound. For example, my
> hb_vmExactlyEqual() code was changed from:
> else if( HB_IS_NUMERIC( pItem1 ) && HB_IS_NUMERIC( pItem2 ) )
> hb_vmPushLogical( hb_vmPopNumber() == hb_vmPopNumber() );
> ...to this...
> else if( HB_IS_NUMERIC( pItem1 ) && HB_IS_NUMERIC( pItem2 ) )
> {
> double dNumber2 = hb_vmPopNumber();
> double dNumber1 = hb_vmPopNumber();
> dNumber1 = hb_numRound( dNumber1, (int) MY_PRECISION ); //
> MY_PRECISION is defined as 8
> dNumber2 = hb_numRound( dNumber2, (int) MY_PRECISION ); //
> MY_PRECISION is defined as 8
> hb_vmPushLogical( dNumber1 == dNumber2 );
^^
You are adding workaround for float point arithmetic problems
hardcoding exact comparission so it's still wrong. The above
code should be changed to:
/* define precision factor which will reduce precision
* to ~15 digit but it will work for any range of supported
* values.
*/
#define HB_PRECISION_FACTOR 1.000000000000005
[...]
else if( HB_IS_NUMERIC( pItem1 ) && HB_IS_NUMERIC( pItem2 ) )
{
double dNumber2 = hb_vmPopNumber();
double dNumber1 = hb_vmPopNumber();
BOOL fResult;
if( dNumber1 < dNumber2 )
{
if( dNumber1 < 0 )
fResult = dNumber2 * HB_PRECISION_FACTOR <= dNumber1;
else
fResult = dNumber1 * HB_PRECISION_FACTOR >= dNumber2;
}
else
{
if( dNumber1 < 0 )
fResult = dNumber1 * HB_PRECISION_FACTOR <= dNumber2;
else
fResult = dNumber2 * HB_PRECISION_FACTOR >= dNumber1;
}
hb_vmPushLogical( fResult );
}
This code will be probably faster, it will work for any supported range
numbers and it's not effected by compiler/hardware floating point
arithmetic. The cost is reducing precision to ~15 significant decimal
digits. So the cumulative difference in your code during calculations
should not be bigger. If it's too small for you then increase
HB_PRECISION_FACTOR removing one or more 0. It will also reduce the
precision.
> Again, this works fine in MSVC v6 but the following PRG code fails if
> I build this using VS 2005:
> ? 3.2 == 3.2 // returns .T. in MSVC v6 and .F. in VS 2005.
Nothing amazing - it's expected in such code. You still have FL
comparison problem.
> If I remove the calls to hn_numRound() as...
> else if( HB_IS_NUMERIC( pItem1 ) && HB_IS_NUMERIC( pItem2 ) )
> {
> double dNumber2 = hb_vmPopNumber();
> double dNumber1 = hb_vmPopNumber();
> // dNumber1 = hb_numRound( dNumber1, (int) MY_PRECISION );
> // dNumber2 = hb_numRound( dNumber2, (int) MY_PRECISION );
> hb_vmPushLogical( dNumber1 == dNumber2 );
> }
> ...it works ok.
As above. It may work for this number but it may not work for other.
> Can anyone tell my why there is a difference between these 2 C versions?
It's FL arithmetic nature. These two compilers uses different floating
point math libraries or you build Harbour with different math optimization
switches. Such difference can appear even with the same binary program
but executed on different machines due to small difference between
math coprocessors in different CPUs. As above nothing amazing.
You have two choices:
1. check carefully for MSVC math optimization switches used to build
harbour and disable them. It may help for some chosen cases.
2. change the method used to compare numbers, f.e. to the one
I presented but please check me - I've just written this code
by finger without any tests and it's possible that I made some
mistakes but I hope you understand the idea - it's important
because my version behaves differently then yours.
If you do not understand it and you are happy with your current
code then simply change it to:
else if( HB_IS_NUMERIC( pItem1 ) && HB_IS_NUMERIC( pItem2 ) )
{
double dNumber2 = hb_vmPopNumber();
double dNumber1 = hb_vmPopNumber();
hb_vmPushLogical( hb_numRound( dNumber1 - dNumber2,
(int) MY_PRECISION ) == 0 );
}
best regards,
Przemek
_______________________________________________
Harbour mailing list
Harbour@harbour-project.org
http://lists.harbour-project.org/mailman/listinfo/harbour
