Skip to content

Fix #12143: Improve precision of rounding for FP numbers #12159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Fix #12143: Improve precision of rounding for FP numbers #12159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c474eb
Change precision of rounding for large FP numbers:
jorgsowa Sep 8, 2023
efd8e7d
Fix rounding for large precision FP numbers for PHP_ROUND
jorgsowa Sep 8, 2023
112d9d4
Improve the rounding for FP numbers for modes PHP_ROUND_HALF_ODD and …
jorgsowa Sep 8, 2023
f762b8b
Removed changes to prerounding
jorgsowa Sep 9, 2023
f043c10
Added more numbers to test
jorgsowa Sep 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions ext/standard/math.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,17 @@ static inline double php_round_helper(double value, int mode) {
tmp_value = floor(value + 0.5);
if ((mode == PHP_ROUND_HALF_DOWN && value == (-0.5 + tmp_value)) ||
(mode == PHP_ROUND_HALF_EVEN && value == (0.5 + 2 * floor(tmp_value/2.0))) ||
(mode == PHP_ROUND_HALF_ODD && value == (0.5 + 2 * floor(tmp_value/2.0) - 1.0)))
(mode == PHP_ROUND_HALF_ODD && value == (0.5 + 2 * floor(tmp_value/2.0) - 1.0)) ||
value < (-0.5 + tmp_value))
{
tmp_value = tmp_value - 1.0;
}
} else {
tmp_value = ceil(value - 0.5);
if ((mode == PHP_ROUND_HALF_DOWN && value == (0.5 + tmp_value)) ||
(mode == PHP_ROUND_HALF_EVEN && value == (-0.5 + 2 * ceil(tmp_value/2.0))) ||
(mode == PHP_ROUND_HALF_ODD && value == (-0.5 + 2 * ceil(tmp_value/2.0) + 1.0)))
(mode == PHP_ROUND_HALF_ODD && value == (-0.5 + 2 * ceil(tmp_value/2.0) + 1.0)) ||
value > (0.5 + tmp_value))
{
tmp_value = tmp_value + 1.0;
}
Expand All @@ -131,7 +133,7 @@ PHPAPI double _php_math_round(double value, int places, int mode) {
return value;
}

places = places < INT_MIN+1 ? INT_MIN+1 : places;
places = places < INT_MIN + 1 ? INT_MIN + 1 : places;
precision_places = 14 - php_intlog10abs(value);

f1 = php_intpow10(abs(places));
Expand All @@ -140,7 +142,7 @@ PHPAPI double _php_math_round(double value, int places, int mode) {
the requested places BUT is small enough to make sure a non-zero value
is returned, pre-round the result to the precision */
if (precision_places > places && precision_places - 15 < places) {
int64_t use_precision = precision_places < INT_MIN+1 ? INT_MIN+1 : precision_places;
int64_t use_precision = precision_places < INT_MIN + 1 ? INT_MIN + 1 : precision_places;

f2 = php_intpow10(abs((int)use_precision));
if (use_precision >= 0) {
Expand All @@ -153,7 +155,7 @@ PHPAPI double _php_math_round(double value, int places, int mode) {
tmp_value = php_round_helper(tmp_value, mode);

use_precision = places - precision_places;
use_precision = use_precision < INT_MIN+1 ? INT_MIN+1 : use_precision;
use_precision = use_precision < INT_MIN + 1 ? INT_MIN + 1 : use_precision;
/* now correctly move the decimal point */
f2 = php_intpow10(abs((int)use_precision));
/* because places < precision_places */
Expand Down
38 changes: 38 additions & 0 deletions ext/standard/tests/math/round_bug12143.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
Bug #12143 round() with large FP
--FILE--
<?php
var_dump(round(0.49999999999999994, 0, PHP_ROUND_HALF_UP));
var_dump(round(0.49999999999999994, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(0.49999999999999994, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(0.49999999999999994, 0, PHP_ROUND_HALF_ODD));
var_dump(round(-0.49999999999999994, 0, PHP_ROUND_HALF_UP));
var_dump(round(-0.49999999999999994, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(-0.49999999999999994, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(-0.49999999999999994, 0, PHP_ROUND_HALF_ODD));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_UP));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_ODD));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_UP));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_ODD));
Comment on lines +13 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[IMO]

The internal representation of double precision around 0.5 in IEEE754 and the corresponding FP are as follows:

3fdfffffffffffff // 0.49999999999999994
3fe0000000000000 // 0.5
3fe0000000000001 // 0.50000000000000001

Therefore, considering the purpose of the test, it would be better to set the value as follows(There were some places where digits were missing):

Suggested change
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_UP));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(0.5000000000000004, 0, PHP_ROUND_HALF_ODD));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_UP));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(-0.5000000000000004, 0, PHP_ROUND_HALF_ODD));
var_dump(round(0.5, 0, PHP_ROUND_HALF_UP));
var_dump(round(0.5, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(0.5, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(0.5, 0, PHP_ROUND_HALF_ODD));
var_dump(round(-0.5, 0, PHP_ROUND_HALF_UP));
var_dump(round(-0.5, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(-0.5, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(-0.5, 0, PHP_ROUND_HALF_ODD));
var_dump(round(0.50000000000000001, 0, PHP_ROUND_HALF_UP));
var_dump(round(0.50000000000000001, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(0.50000000000000001, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(0.50000000000000001, 0, PHP_ROUND_HALF_ODD));
var_dump(round(-0.50000000000000001, 0, PHP_ROUND_HALF_UP));
var_dump(round(-0.50000000000000001, 0, PHP_ROUND_HALF_DOWN));
var_dump(round(-0.50000000000000001, 0, PHP_ROUND_HALF_EVEN));
var_dump(round(-0.50000000000000001, 0, PHP_ROUND_HALF_ODD));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also want to check 1.5 and 2.5 as well to confirm the operation of ODD and EVEN.

[FYI]

These internal representations and their corresponding FP are:

3ff7ffffffffffff // 1.4999999999999998
3ff8000000000000 // 1.5
3ff8000000000001 // 1.5000000000000002

4003ffffffffffff // 2.4999999999999996
4004000000000000 // 2.5
4004000000000001 // 2.5000000000000004

?>
--EXPECT--
float(0)
float(0)
float(0)
float(0)
float(0)
float(0)
float(0)
float(0)
float(1)
float(1)
float(1)
float(1)
float(-1)
float(-1)
float(-1)
float(-1)