Hi,
A few weeks ago I wrote quite a long posting to internals@ that tried to
clarify the situation on the round() function in PHP. I was asked to
write it up as an RFC in the wiki, which I have done:
http://wiki.php.net/rfc/rounding
In the mean time, I had some time to think about that problem again and
am quite sure that I have a feasible solution for the problem. The RFC
includes a patch that addresses all problems and is in my eyes the best
solution. The patch is actually much simpler than the RFC itself. ;-)
I also discovered that zend_strtod() which is responsible for string to
double conversions does not yield the same results as the traditional
strtod() due to the same "extended precision truncated to double
precision" problem on 32 bit x86 systems with GNU compiler. The patch
also addresses that.
Anyway, feel free to comment.
Regards,
Christian
PS: While testing the Windows versions, I discovered, that someone
removed http://www.php.net/extras/win32build.zip - is that intentional?
I got the previous version out of CVS in order to be able to build PHP.
If it is intentional, where do I get updated build instruction when
compared to http://www.php.net/manual/en/install.windows.building.php?
PS: While testing the Windows versions, I discovered, that someone
removed http://www.php.net/extras/win32build.zip - is that intentional?
That was intentional yes. I asked Pierre, and he said nothing from
that folder was used.
I got the previous version out of CVS in order to be able to build PHP.
If it is intentional, where do I get updated build instruction when
compared to http://www.php.net/manual/en/install.windows.building.php?
I honestly didn't even think of grepping the documentations :]
But apparently the wiki is used as documentations these days:
http://wiki.php.net/internals/windows
-Hannes
hi,
PS: While testing the Windows versions, I discovered, that someone
removed http://www.php.net/extras/win32build.zip - is that intentional?
Yes, see for the current versions:
http://wiki.php.net/internals/windows/
http://wiki.php.net/internals/windows/libs
http://pecl2.php.net/downloads/php-windows-builds/php-libs/
Cheers,
Pierre
Hi again,
A few weeks ago I wrote quite a long posting to internals@ that tried to
clarify the situation on theround()function in PHP. I was asked to
write it up as an RFC in the wiki, which I have done:
Since there has been no reaction so far to my proposal (besides the help
from Hannes and Pierre for the Win32 build dependencies, thanks for that
btw.), I'd like to know why? Am I talking gibberish and my proposal is
hard to understand? Or have I simply chosen a topic that many do not
consider worthwhile investing that amount of energy into? Or is it
something else?
Regards,
Christian
Hi again,
A few weeks ago I wrote quite a long posting to internals@ that tried
to clarify the situation on theround()function in PHP. I was asked
to write it up as an RFC in the wiki, which I have done:Since there has been no reaction so far to my proposal (besides the
help from Hannes and Pierre for the Win32 build dependencies, thanks
for that btw.), I'd like to know why? Am I talking gibberish and my
proposal is hard to understand? Or have I simply chosen a topic that
many do not consider worthwhile investing that amount of energy into?
Or is it something else?
For the record, I was very excited to see your proposal and the work you've done.
I'm not a PHP dev so I can't comment on the patch, maybe there might be a performance concern?
I don't know either the history of the 'rounding fuzz', but there's definitely space for improvement... The problem naturally isn't with floats but that the round() function is very misleading. My opinion is the rounding precision should be removed or fixed since it doesn't work "as expected". i.e. javascript, there's no precision: Math.round()
"
Nevertheless, what could be discussed separately is the introduction of a new type that automatically uses an arbitrary precision library internally, since writing $a * $b is much more natural than e.g. bcmul($a, $b). This, however, goes far beyond the scope of this proposal.
"
I'm all for this... maybe something like this:
// within some 'numeric' precision
$a = (numeric)2;
$b = (numeric)0.5;
$mult = $a * $b;
echo $mult;// 1 (numeric)
echo (2.5 == $a + $b); // true
echo (2.5 === $a + $b); // false
echo ((numeric)2.5 === $a + $b); // true
Hi,
For the record, I was very excited to see your proposal and the work you've done.
Thanks! :)
I'm not a PHP dev so I can't comment on the patch, maybe there might
be a performance concern?
The new algorithm is slightly slower in most of the cases and quite a
bit slower if strings have to be used in order to ensure correctness.
The following table shows the performance comparison of the algorithm
when compiling 5.3 with gcc 4.1.2 on Gentoo Linux with and without
optimization activated:
+----+----+-----------------------+-----------------------+
| | | without patch | with patch |
| st | pr | -O0 -ggdb | -O2 | -O0 -ggdb | -O2 |
+----+----+-----------+-----------+-----------+-----------+
| - | X | 6.427930 | 3.151313 | 7.107897 | 3.581429 |
| X | X | 6.485305 | 3.197008 | 26.423409 | 15.571503 |
| X | - | 6.318853 | 3.124304 | 11.926427 | 6.910457 |
| - | - | 6.100793 | 2.850991 | 6.636300 | 2.926919 |
| precov. | 5.432411 | 2.382959 | 5.524717 | 2.382117 |
+----+----+-----------+-----------+-----------+-----------+
Times are in seconds, script was basically the following:
$start = microtime(true);
for ($i = 0; $i < 5000000; $i++) $ret = round (/number/, /prec/);
$end = microtime(true);
printf("%.6f\n", $end - $start);
Explanations for the left columns:
st: Uses to-string-and-back conversion to ensure correctness (the case
when |places| > 22)
pr: Uses prerounding to precision to ensure correctness (see RFC for
details)
precov: Precision overflow, i.e. rounding 12345.123456789012345 to more
than 10 places precision (which is the max. that doubles can represent)
The most commen case by far would be the first one (things such as
round(12.245, 2)), the cases where string-conversion is used are
probably extremely rare (remember, |second parameter of round| must be >
22).
I used the following numbers to test the combinations:
-st +pr: round (0.116, 2)
+st +pr: round (0.000000000000000000000000000000116, 32)
+st -pr: round (0.000000000000000000000000000000005, 30)
-st -pr: round (0.005, 0)
precov.: round (1, 30)
As you can see, the cases which don't to string-and-back conversion are
not problematic: There's basically no difference for the precision
overflow case, the case with prerounding is slightly slower and the case
without prerounding is actually slightly faster when compiler
optimizations are enabled (this is due to the fact that the 10^places
was optimized in my patch). So the most common cases basically don't
matter much at all.
The only potential problems are the cases where the second parameter of
the round() function is larger than 22 or smaller than -22. In that
case, the new algorithm is quite a bit slower than the old one (nearly
factor 5 for all numbers that won't become 0 after rounding). But these
are edge cases which won't happen that often and if people actually have
these numbers they are probably far more interested in correctness than
in performance [see for example var_dump(2e-23 - round(2e-23,23));].
---------------------------- snip ----------------------------------
@internals: Anyway, since nobody of the core devs seems to be interested
in this topic, is there any objection to me committing this patch to HEAD?
@Johannes, @Lukas: Should there be a decision for an alpha3 release of
PHP 5.3 due to the namespace discussion, is there any objection to me
committing this patch to PHP_5_3? Should there be directly a beta1
release, any objections against this patch in 5.3.1? Or, even though it
is a rewrite of the round() function, may this even qualify as a bugfix,
since the previous behaviour was erroneous (see for example
http://bugs.php.net/bug.php?id=42294 and my RFC)?
Regards,
Christian
Hi,
@internals: Anyway, since nobody of the core devs seems to be interested
in this topic, is there any objection to me committing this patch to HEAD?
Since I received no answer, I assumed that nobody would object and tried
to commit my patch to HEAD. Problem is that I don't have ZE2 karma and
since my patch touches zend_strtod.[ch] too, my commit failed.
I isolated the ZE2 parts of the patch so they can be applied separately
so that somebody with ZE2 karma can commit it:
http://www.christian-seiler.de/temp/php/2008-08-23-rounding/ze2.patch
For an explanation for everyone that hasn't read my rounding RFC, the
ZE2 part of the patch does the following:
- Define some macros in zend_strtod.h that set the FPU control registers
to always use double precision (and not double extended precision)
when performing calculations. - Add necessary configure checks for Unix based systems to see if the
required mechanism is present. - Define the correct constant in the Windows build system (MSVC always
defines _controlfp_s) - Modify zend_strtod() that it uses these macros.
The macros are defined in zend_strtod.h because I also need them in
ext/standard/math.c.
The reason for the need to fix zend_strtod() is that it currently
behaves like strtod() on most systems but like (double)strtold() on x86
UNIX with GCC (see my RFC for details why that's not the same).
For the record, the complete patch I tried to commit (I'll commit the
rest myself):
http://www.christian-seiler.de/temp/php/2008-08-23-rounding/rounding-head.patch
I'd really appreciate it if someone could commit the ZE2 parts. Thanks!
Regards,
Christian