Page 2 of 3
"Round()" error, probably a bug in USUAL/FLOAT to Real8 conversion rather?
Posted: Sun Apr 21, 2019 3:52 pm
by Karl-Heinz
Jamal wrote:
P.S. For my info, may be someone can tell what situation(s) one may one use a negative number in the decimal places parameter in the Round() function. I've never seen it or needed it.
Hi Jamal,
Based on this link, I've created a statistic that uses a negative value to round the number of the population
https://ec.europa.eu/eurostat/statistic ... ad_example
Code: Select all
FUNCTION Statistic() AS VOID
LOCAL aPopulation AS ARRAY
LOCAL dwCurrentDecSep, dwLen, i AS DWORD
aPopulation := {{ "BE" , 9660154 } , {"DE" , 61194591 } , { "IT" , 53685300 } , { "LU" , 338500m } , { "NL" , 12957621 } }
dwLen := alen ( aPopulation )
?
FOR i := 1 UPTO dwLen
? aPopulation [ i , 1 ] + _chr(9) + transform (Round ( aPopulation [ i , 2 ] , -4 ) / 1000 , "999 999" )
NEXT
?
dwCurrentDecSep := setdecimalsep ( asc ( "." ) )
?
FOR i := 1 UPTO dwLen
? aPopulation [ i , 1 ] + _chr(9) + transform ( ( Round ( aPopulation [ i , 2 ] , -4 ) / 10000 ) / 100.00 , "9999.99" )
NEXT
setdecimalsep ( dwCurrentDecSep )
?
RETURN
regards
Karl-Heinz
"Round()" error, probably a bug in USUAL/FLOAT to Real8 conversion rather?
Posted: Sun Apr 21, 2019 4:05 pm
by Karl-Heinz
Upps,
just noticed that in the array the number "338500m" still contains the "m" - it was just a test , that i forgot to remove
"Round()" error, probably a bug in USUAL/FLOAT to Real8 conversion rather?
Posted: Sun Apr 21, 2019 4:09 pm
by Karl-Heinz
Hi Chris/Robert,
will the Decimal type handling happen in a Round() overload ?
FUNCTION Round ( nNumber AS DECIMAL , siDecimals AS INT ) AS DECIMAL
regards
Karl-Heinz
"Round()" error, time to resolve this forever
Posted: Mon Apr 22, 2019 12:19 am
by Chris
lumberjack wrote:Chris,
Chris wrote:But, remember, VO does not have a decimal type, but still returns the expected results in this area, with REAL and FLOAT data types and of course there is also so much existing code that we need to support.
I am a bit confused, why:
"VO does not have a decimal type"? Surely we now in X# with VO language support, and the aim is to not use VO anymore, but X#?
I can't imagine anybody wanting a banker's rounding when RoundAwayFromZero is specified when a number cannot be exactly represented...
Johan, please read my posts above, the problem does not have to do with banker's rounding, it's just the inherent insufficient precision of the Double/REAL/FLOAT data types when representing decimal numbers.
We need to support properly all the VO data types, because there are millions of lines of existing code that uses them, we cannot simply ask people to go in a million places and change their code to now use the Decimal data type, make infinite amount of tests to make sure they still get the same results etc.
But, I agree, for new code, everywhere we need to use decimal numbers and there are no critical performance considerations, then we should always be using System.Decimal.
"Round()" error, probably a bug in USUAL/FLOAT to Real8 conversion rather?
Posted: Mon Apr 22, 2019 12:23 am
by Chris
Hi Karl-Heinz,
Karl-Heinz wrote:
will the Decimal type handling happen in a Round() overload ?
FUNCTION Round ( nNumber AS DECIMAL , siDecimals AS INT ) AS DECIMAL
Well, this overload would internally use Decimal.Round() instead of Math.Round(), so I assume it wouln't have that problem. But probably isn't it better to always use Decimal.Round() with Decimal vars directly, instead of introducing a runtime functions which simply just calls that?
"Round()" error, time to resolve this forever
Posted: Mon Apr 22, 2019 6:29 am
by lumberjack
Chris wrote:but we need to check if the speed penalty is too much and if it indeed works well in all cases
Well, as they say the proof of the pudding, is in the X# testing...:
Code: Select all
FUNCTION Start() AS VOID
LOCAL u := 5.0 AS USUAL
LOCAL y AS USUAL
LOCAL z AS REAL8
LOCAL dDec, dMDiff, dVDiff AS Decimal
SetFloatDelta(10)
SetDecimal(9)
? PadL("USUAL", 15), PadL("REAL8", 15), PadL("ROUND", 5), ;
PadL("Dec Round", 15), PadL("MathRound", 15), PadL("VORound", 15), "MathFact", "VOFact"
FOR LOCAL x := 0 TO 9
FOR LOCAL i := 1 UPTO 9
y := x + ( u * 10^(-i))
z := x + ( 5.0 * 10^(-i))
? PadL(y, 15), PadL(z, 15), PadL(i - 1, 5), ;
PadL(dDec := Decimal.Round((Decimal)(REAL8)y, i - 1, MidpointRounding.AwayFromZero), 15), ;
PadL(dMDiff := Math.Round((REAL8)y, i - 1, MidpointRounding.AwayFromZero), 15), ;
PadL(dVDiff := VORound(y, i-1), 15), PadL(dMDiff / dDec, 8), PadL(dVDiff / dDec, 6)
NEXT
NEXT
BenchMark()
RETURN
FUNCTION VORound(r8 AS REAL8, iDec AS INT) AS REAL8
VAR dRound := Decimal.Round((Decimal)r8, iDec, MidpointRounding.AwayFromZero)
RETURN (REAL8)dRound
FUNCTION BenchMark() AS VOID
LOCAL u := 5.0 AS REAL8
LOCAL dStart AS DateTime
LOCAL mTick, vTick AS TimeSpan
dStart := DateTime.Now
FOR LOCAL j := 1 UPTO 10000000
FOR LOCAL i := 1 UPTO 9
Math.Round(u, i - 1, MidpointRounding.AwayFromZero)
NEXT
NEXT
? "Math.Round", (mTick := DateTime.Now - dStart):ToString()
dStart := DateTime.Now
FOR LOCAL j := 1 UPTO 10000000
FOR LOCAL i := 1 UPTO 9
VORound(u, i - 1)
NEXT
NEXT
? " VORound", (vTick := DateTime.Now - dStart):ToString()
? " Penalty", (Decimal)vTick:Ticks / (Decimal)mTick:Ticks
RETURN
"Round()" error final Test
Posted: Mon Apr 22, 2019 9:53 am
by lumberjack
Final testing:
Code: Select all
FUNCTION Start() AS VOID
LOCAL u := 5.0 AS USUAL
LOCAL y AS USUAL
LOCAL z AS REAL8
LOCAL dDec, dMDiff, dVDiff AS Decimal
SetFloatDelta(10)
SetDecimal(9)
? PadL("USUAL", 15), PadL("REAL8", 15), PadL("ROUND", 5), ;
PadL("Dec Round", 15), PadL("MathRound", 15), PadL("VORound", 15), "MathFact", "VOFact"
FOR LOCAL x := 0 AS INT TO 9
FOR LOCAL i := 1 AS INT UPTO 9
y := x + ( u * 10^(-i))
z := x + ( 5.0 * 10^(-i))
? PadL(y, 15), PadL(z, 15), PadL(i - 1, 5), ;
PadL(dDec := Decimal.Round((Decimal)(REAL8)y, i - 1, MidpointRounding.AwayFromZero), 15), ;
PadL(dMDiff := (Decimal)(REAL8)Round(y, i - 1), 15), ;
PadL(dVDiff := VORound(y, i-1), 15), PadL(dMDiff - dDec, 8), PadL(dVDiff - dDec, 6)
NEXT
NEXT
BenchMark()
RETURN
FUNCTION VORound(r8 AS REAL8, iDec AS INT) AS REAL8
VAR dRound := Decimal.Round((Decimal)r8, iDec, MidpointRounding.AwayFromZero)
RETURN (REAL8)dRound
FUNCTION BenchMark() AS VOID
LOCAL u := 5.0 AS USUAL
LOCAL dStart AS DateTime
LOCAL vTick, mTick, xTick AS TimeSpan
dStart := DateTime.Now
FOR LOCAL j := 1 AS INT UPTO 10000000
FOR LOCAL i := 1 AS INT UPTO 9
Round(u, i - 1)
NEXT
NEXT
? " VO.Round", (vTick := DateTime.Now - dStart):ToString()
dStart := DateTime.Now
FOR LOCAL j := 1 AS INT UPTO 10000000
FOR LOCAL i := 1 AS INT UPTO 9
Math.Round((REAL8)u, i - 1)
NEXT
NEXT
? "Math.Round", (mTick := DateTime.Now - dStart):ToString()
dStart := DateTime.Now
FOR LOCAL j := 1 AS INT UPTO 10000000
FOR LOCAL i := 1 AS INT UPTO 9
VORound(u, i - 1)
NEXT
NEXT
? " Dec.Round", (xTick := DateTime.Now - dStart):ToString()
? " Penalty", (Decimal)vTick:Ticks / (Decimal)mTick:Ticks, (Decimal)xTick:Ticks / mTick:Ticks
RETURN
/************** Results **************
VO.Round 00:00:05.9704984
Math.Round 00:00:03.1099580
Dec.Round 00:00:10.2371290
Penalty 1.919800332 3.291725805
************ End of Results **********/
"Round()" error final Test
Posted: Mon Apr 22, 2019 11:15 am
by robert
Johan,
I am not sure what exactly your idea behind this message is ?
- Are you suggesting that we replace the current Round() function with your VORound() ?
- Or something else ?
And your code tests the speed, but does it also test the correctness ?
Decimals are limited to 4 positions. Does your code work for Round() with more than 4 positions ?
Without additional comments I don't know what to do with your message.
Robert
"Round()" error final Test
Posted: Mon Apr 22, 2019 11:19 am
by Chris
Thanks Johan! So there's a considerable performance penalty with Decimal.Round(), but it's not a massive one, and since it is very important to have the same results as in VO, I think we should go with it. Just need to make sure that it does indeed work as expected in all cases, but in my tests so for it seems to do so.
"Round()" error final Test
Posted: Mon Apr 22, 2019 11:25 am
by Chris
robert wrote:Johan,
I am not sure what exactly your idea behind this message is ?
- Are you suggesting that we replace the current Round() function with your VORound() ?
- Or something else ?
And your code tests the speed, but does it also test the correctness ?
Decimals are limited to 4 positions. Does your code work for Round() with more than 4 positions ?
Without additional comments I don't know what to do with your message.
Robert
Robert, I think Johan was responding to the "speed considerations" I posted earlier. Apparently the speed difference between Math.Round() and Decimal.Round() is not too huge (including the conversions to/from Decimal), so I think we should go with it (adjust the Round() function in the runtime to use Math.Round()). I have done some tests about correctness and it seems to work fin, but will do some more of course.