We upgraded to 1.8 a couple of weeks ago, today i had to investigate this little gem:
The issue was reproducable by adding 3 of the above items to the cart, checking the db confirmed that the 2nd quote item already had the wrong tax value.
Digging down the culprit turned out to be in Mage_Tax_Model_Sales_Total_Quote_Subtotal and Mage_Tax_Model_Sales_Total_Quote_Tax:
collect() will call for each quote item:
- _processItem() which calls:
- _totalBaseCalculation() which then uses a custom rounding function:
- _deltaRound($price, $rate, $direction, $type = 'regular')
This method applies a delta to the input price before rounding it, starting with a default delta of 0.
$this->_roundingDeltas[$type][$rate] = $price - $this->_calculator->round($price)Should another call to _deltaRound() use the same type and rate parameters the function will apply the delta from the previous call.
But lets see what actually happens:
1st item added to cart, price is 19.90 incl. tax:
- tax calculated: 3.1773109243697
- delta value: 0
- rounded tax value: 3.18
2nd item added to cart, price is 19.90 incl. tax:
- tax calculated: 3.1773109243697
- delta value: -0.0026890756302529
- tax before rounding: 3.1746218487395
- rounded tax value: 3.17
3rd item added to cart, price is 19.90 incl. tax:
- tax calculated: 3.1773109243697
- delta value: 0.0046228487394946
- tax before rounding: 3.1819337731092
- rounded tax value: 3.18
I don't really get the point of this function, only thing i can think of is working around the precision problems of using floats. Maybe it is just used in the wrong way as i don't see why the rounding delta of a totally different item should matter for rounding another one. Tested it with couple other item combinations, the more items you add to the cart the higher the chance of getting a wrong tax value at some point.
Upgraded to 1.8.1, _deltaRounding() got some love but they only changed the default delta value to 0.000001 to work around the trollish behaviour of php's round() when handling x.5 values. The issue still persists.
As a quick fix you could modify _deltaRounding() in both classes to always use the default delta which fixes this specific issue, no idea what side effects this might cause though.
Edit: I think i get the point of this function now. Looks like it's for working around the fact that Magento saves all money values already rounded to 2 digits. Those value are then used for totals calculation which leads to errors.
By always using the default delta you end up with correct rounded values for the tax per position, but the total tax will be wrong:
3.18 * 3 = 9.54 total item tax in backend after fix
3.1773109243697 * 3 = 9.53193577311 or 9.53 rounded
Without the fix some of the rounded tax values per position can be wrong, but it will produce the correct final tax amount. IMHO all this mess could be avoided by not saving already rounded money values into the db, atleast not rounded to 2 digits.
In our case the connected erp will use the potential wrong values of row totals and barf all over it. Will probably just fix the api method to feed correct values to the erp.
Re:
I don't really get the point of this function, only thing i can think of is working around the precision problems of using floats.It distributes rounding error amongst the data. Consider an input continually made up of 4/3. You can round the normal way
{1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33}But now you have a cumulate error. If you want the total to match the original total you need to place it somewhere. For example on the end:
{1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.33, 1.36}This is often done in things like billing where you final or initial bill will be different.
Or you can distribute the error as you go
{1.33, 1.33, 1.34, 1.33, 1.33, 1.34, 1.33, 1.33, 1.34}At any point in time your total error is kept small, but it's a bit of a pain if your monthly bill keeps changing.