You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@velocity.apache.org by David Parks <da...@yahoo.com> on 2010/11/03 12:00:00 UTC
Decimal and rounding error - two problems
I ran into a problem with decimal formats. Example:
$math.roundTo(10, 0.158343834599)
Actual Result : 0.15834383478471944
Expected result : 0.1583438346
Aside from roundTo(...) producing incorrect numbers of decimal places (when
10 or more is entered as param #1) the rounding is blatantly incorrect (I
expect this is due to Decimal rounding error).
I wanted to fix this by writing my own methods to use BigDecimal, but here I
ran into a problem. When I implemented:
public Number myRoundTo(int decimalPlaces, String num){
BigDecimal bg = new BigDecimal(num);
return bg.setScale(decimalPlaces, RoundingMode.HALF_UP);
}
A call to $mymath.myRoundTo(2, 1.234) is never called. This is because 1.234
is converted to a Decimal and velocity finds no method with a signature of a
(Integer, Decimal) (I can accept an Object, but then I still get a Decimal
which will produce rounding error as documented in BigDecimal javadocs). My
concerns are:
1) Why can't velocity convert a number parameter to a String or BigDecimal
format if the method signature calls for that. This should not be complex
using reflection. If someone will even point me to the right spot in the
code I might be able to post a quick patch for it.
2) roundTo(...) should be fixed to avoid Decimal use, this unfortunately
seems to depend on #1. In fact Decimals should be avoided altogether for
view related cases, the performance gain over BigDecimal just isn't going to
be notable in this context.
3) Why convert a number - like the 2nd param in: $math.roundTo(3, 1.234) -
to a Decimal rather than a BigDecimal? Seems more logical to convert it to
BigDecimal then pass it to the method as a Number.
For now my solution is the inelegant:
#mymath.myRoundTo(2, "1.234")
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
Re: Decimal and rounding error - two problems
Posted by Nathan Bubna <nb...@gmail.com>.
I'm open to patches for making MathTool's roundTo correct. And i
might move the quick/dirty roundTo to DisplayTool. So users just
using it to format things for display can still access a "lighter
version".
And if you don't like passing numbers as strings, you can always have
your custom tool methods accept Object and do the conversion yourself.
As Claude says, we are limited in the assumptions we can safely make.
On Wed, Nov 3, 2010 at 6:51 AM, David Parks <da...@yahoo.com> wrote:
> Hi Claude, thanks for the two very quick responses. I'm going to write up
> some comments for the XML based on the info you provided and send it in as
> an enhancement to the website.
>
> On the rounding, yes, generally this works for smaller numbers, that's
> mainly because the toString() of Double truncates the error at the end of
> Double (thus Double(0.1).toString() produces 0.1, not the actual binary
> representation of 0.1, which is
> 0.1000000000000000055511151231257827021181583404541015625).
>
> To respond to your comment:
> "I'm not convinced that we should use BigDecimals when unnecessary - what
> would we gain?"
>
> By not using BigDecimal we are assuming all users will use small numbers for
> which Double.toString() formats nicely. This is an unsafe assumption (as I
> found out in literally my first use of the Tools). We would gain by doing
> away with this unsafe assumption and always guaranteeing correct results
> regardless of how many decimal places the user enters. I don't see a
> downside of using BigDecimal. It's unlikely that users are doing heavy duty
> math computations in a view rendering component, the only place where I see
> a benefit to using Double over BigDecimal. We should code for correctness
> and no surprises.
>
> My wishlist on this one:
> - Replace Double roundTo(Object, Object) with
> BigDecimal roundTo(Object, Object) -> Defaults to RoundingMode.HALF_UP
> (and document this)
> BigDecimal roundTo(Object, Object, String roundingMode) (give us
> rounding options from java.math.RoundingMode)
> - Parse decimal numbers to the safe BigDecimal rather than Double (
> #math.roundTo(2, 1.234), 1.234 should be passed to the MathTool class as a
> BigDecimal )
>
> I'd be happy to do this and post the update as an enhancement if it's a
> direction the committers think is valid. I'd need a little help figuring out
> the code for the default conversion of numbers to BigDecimal rather than
> Double (which has the potential to cause unexpected results). The MathTool
> code is trivial and I've done it already for myself.
>
> Dave
>
>
>
> -----Original Message-----
> From: Claude Brisson [mailto:claude@renegat.net]
> Sent: Wednesday, November 03, 2010 8:09 PM
> To: Velocity Users List
> Subject: Re: Decimal and rounding error - two problems
>
> On 03/11/2010 12:00, David Parks wrote:
>> I ran into a problem with decimal formats. Example:
>>
>> $math.roundTo(10, 0.158343834599)
>> Actual Result : 0.15834383478471944
>> Expected result : 0.1583438346
>>
>>
> According to its souce code, the roundTo method is, for the time being,
> limited to 9 digits.
>> Aside from roundTo(...) producing incorrect numbers of decimal places
>> (when 10 or more is entered as param #1) the rounding is blatantly
>> incorrect (I expect this is due to Decimal rounding error).
>>
>> I wanted to fix this by writing my own methods to use BigDecimal, but
>> here I ran into a problem. When I implemented:
>>
>> public Number myRoundTo(int decimalPlaces, String num){
>> BigDecimal bg = new BigDecimal(num);
>> return bg.setScale(decimalPlaces, RoundingMode.HALF_UP);
>> }
>>
>> A call to $mymath.myRoundTo(2, 1.234) is never called. This is because
>> 1.234 is converted to a Decimal and velocity finds no method with a
>> signature of a (Integer, Decimal) (I can accept an Object, but then I
>> still get a Decimal which will produce rounding error as documented in
>> BigDecimal javadocs). My concerns are:
>>
>> 1) Why can't velocity convert a number parameter to a String or
>> BigDecimal format if the method signature calls for that. This should
>> not be complex using reflection. If someone will even point me to the
>> right spot in the code I might be able to post a quick patch for it.
>>
> We are already using reflection to automatize several conversions - but
> conversions between numbers and strings are not always done magically.
> As you found out, quoting numbers is an easy way to convert them into
> strings.
>> 2) roundTo(...) should be fixed to avoid Decimal use, this
>> unfortunately seems to depend on #1. In fact Decimals should be
>> avoided altogether for view related cases, the performance gain over
>> BigDecimal just isn't going to be notable in this context.
>>
> We can't make asumptions on the type of encountered numbers - neither can we
> impose one subclass of Number or the other.
> What we need to is to use BigDecimals when necessary and to fix the MathTool
> to reach a consistent behaviour with BigDecimals.
>> 3) Why convert a number - like the 2nd param in: $math.roundTo(3,
>> 1.234) - to a Decimal rather than a BigDecimal? Seems more logical to
>> convert it to BigDecimal then pass it to the method as a Number.
>>
>>
> I'm not convinced that we should use BigDecimals when unnecessary - what
> would we gain?
>> For now my solution is the inelegant:
>>
>> #mymath.myRoundTo(2, "1.234")
>>
>>
>>
>
> $math.roundTo(2, 1.234) produces 1.23 as expected.
>
>
> Claude
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
> For additional commands, e-mail: user-help@velocity.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
> For additional commands, e-mail: user-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
RE: Decimal and rounding error - two problems
Posted by David Parks <da...@yahoo.com>.
Hi Claude, thanks for the two very quick responses. I'm going to write up
some comments for the XML based on the info you provided and send it in as
an enhancement to the website.
On the rounding, yes, generally this works for smaller numbers, that's
mainly because the toString() of Double truncates the error at the end of
Double (thus Double(0.1).toString() produces 0.1, not the actual binary
representation of 0.1, which is
0.1000000000000000055511151231257827021181583404541015625).
To respond to your comment:
"I'm not convinced that we should use BigDecimals when unnecessary - what
would we gain?"
By not using BigDecimal we are assuming all users will use small numbers for
which Double.toString() formats nicely. This is an unsafe assumption (as I
found out in literally my first use of the Tools). We would gain by doing
away with this unsafe assumption and always guaranteeing correct results
regardless of how many decimal places the user enters. I don't see a
downside of using BigDecimal. It's unlikely that users are doing heavy duty
math computations in a view rendering component, the only place where I see
a benefit to using Double over BigDecimal. We should code for correctness
and no surprises.
My wishlist on this one:
- Replace Double roundTo(Object, Object) with
BigDecimal roundTo(Object, Object) -> Defaults to RoundingMode.HALF_UP
(and document this)
BigDecimal roundTo(Object, Object, String roundingMode) (give us
rounding options from java.math.RoundingMode)
- Parse decimal numbers to the safe BigDecimal rather than Double (
#math.roundTo(2, 1.234), 1.234 should be passed to the MathTool class as a
BigDecimal )
I'd be happy to do this and post the update as an enhancement if it's a
direction the committers think is valid. I'd need a little help figuring out
the code for the default conversion of numbers to BigDecimal rather than
Double (which has the potential to cause unexpected results). The MathTool
code is trivial and I've done it already for myself.
Dave
-----Original Message-----
From: Claude Brisson [mailto:claude@renegat.net]
Sent: Wednesday, November 03, 2010 8:09 PM
To: Velocity Users List
Subject: Re: Decimal and rounding error - two problems
On 03/11/2010 12:00, David Parks wrote:
> I ran into a problem with decimal formats. Example:
>
> $math.roundTo(10, 0.158343834599)
> Actual Result : 0.15834383478471944
> Expected result : 0.1583438346
>
>
According to its souce code, the roundTo method is, for the time being,
limited to 9 digits.
> Aside from roundTo(...) producing incorrect numbers of decimal places
> (when 10 or more is entered as param #1) the rounding is blatantly
> incorrect (I expect this is due to Decimal rounding error).
>
> I wanted to fix this by writing my own methods to use BigDecimal, but
> here I ran into a problem. When I implemented:
>
> public Number myRoundTo(int decimalPlaces, String num){
> BigDecimal bg = new BigDecimal(num);
> return bg.setScale(decimalPlaces, RoundingMode.HALF_UP);
> }
>
> A call to $mymath.myRoundTo(2, 1.234) is never called. This is because
> 1.234 is converted to a Decimal and velocity finds no method with a
> signature of a (Integer, Decimal) (I can accept an Object, but then I
> still get a Decimal which will produce rounding error as documented in
> BigDecimal javadocs). My concerns are:
>
> 1) Why can't velocity convert a number parameter to a String or
> BigDecimal format if the method signature calls for that. This should
> not be complex using reflection. If someone will even point me to the
> right spot in the code I might be able to post a quick patch for it.
>
We are already using reflection to automatize several conversions - but
conversions between numbers and strings are not always done magically.
As you found out, quoting numbers is an easy way to convert them into
strings.
> 2) roundTo(...) should be fixed to avoid Decimal use, this
> unfortunately seems to depend on #1. In fact Decimals should be
> avoided altogether for view related cases, the performance gain over
> BigDecimal just isn't going to be notable in this context.
>
We can't make asumptions on the type of encountered numbers - neither can we
impose one subclass of Number or the other.
What we need to is to use BigDecimals when necessary and to fix the MathTool
to reach a consistent behaviour with BigDecimals.
> 3) Why convert a number - like the 2nd param in: $math.roundTo(3,
> 1.234) - to a Decimal rather than a BigDecimal? Seems more logical to
> convert it to BigDecimal then pass it to the method as a Number.
>
>
I'm not convinced that we should use BigDecimals when unnecessary - what
would we gain?
> For now my solution is the inelegant:
>
> #mymath.myRoundTo(2, "1.234")
>
>
>
$math.roundTo(2, 1.234) produces 1.23 as expected.
Claude
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
Re: Decimal and rounding error - two problems
Posted by Claude Brisson <cl...@renegat.net>.
On 03/11/2010 12:00, David Parks wrote:
> I ran into a problem with decimal formats. Example:
>
> $math.roundTo(10, 0.158343834599)
> Actual Result : 0.15834383478471944
> Expected result : 0.1583438346
>
>
According to its souce code, the roundTo method is, for the time being,
limited to 9 digits.
> Aside from roundTo(...) producing incorrect numbers of decimal places (when
> 10 or more is entered as param #1) the rounding is blatantly incorrect (I
> expect this is due to Decimal rounding error).
>
> I wanted to fix this by writing my own methods to use BigDecimal, but here I
> ran into a problem. When I implemented:
>
> public Number myRoundTo(int decimalPlaces, String num){
> BigDecimal bg = new BigDecimal(num);
> return bg.setScale(decimalPlaces, RoundingMode.HALF_UP);
> }
>
> A call to $mymath.myRoundTo(2, 1.234) is never called. This is because 1.234
> is converted to a Decimal and velocity finds no method with a signature of a
> (Integer, Decimal) (I can accept an Object, but then I still get a Decimal
> which will produce rounding error as documented in BigDecimal javadocs). My
> concerns are:
>
> 1) Why can't velocity convert a number parameter to a String or BigDecimal
> format if the method signature calls for that. This should not be complex
> using reflection. If someone will even point me to the right spot in the
> code I might be able to post a quick patch for it.
>
We are already using reflection to automatize several conversions - but
conversions between numbers and strings are not always done magically.
As you found out, quoting numbers is an easy way to convert them into
strings.
> 2) roundTo(...) should be fixed to avoid Decimal use, this unfortunately
> seems to depend on #1. In fact Decimals should be avoided altogether for
> view related cases, the performance gain over BigDecimal just isn't going to
> be notable in this context.
>
We can't make asumptions on the type of encountered numbers - neither
can we impose one subclass of Number or the other.
What we need to is to use BigDecimals when necessary and to fix the
MathTool to reach a consistent behaviour with BigDecimals.
> 3) Why convert a number - like the 2nd param in: $math.roundTo(3, 1.234) -
> to a Decimal rather than a BigDecimal? Seems more logical to convert it to
> BigDecimal then pass it to the method as a Number.
>
>
I'm not convinced that we should use BigDecimals when unnecessary - what
would we gain?
> For now my solution is the inelegant:
>
> #mymath.myRoundTo(2, "1.234")
>
>
>
$math.roundTo(2, 1.234) produces 1.23 as expected.
Claude
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org