Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.
Issues:
-
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well. - large EG(precision)/PG(serialize_precision) prints meaningless values.
This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.
https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455
This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless values.
This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!
It's been 2 weeks and no comment for this.
I'll start voting.
Thanks.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
----- Original Message -----
From: "Yasuo Ohgaki"
Sent: Friday, September 18, 2015
Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless values.
This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!It's been 2 weeks and no comment for this.
I'll start voting.
Cool, but there's no voting available...? :-)
Thanks.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
- Matt
Hi Yasuo
2015-09-18 23:28 GMT+02:00 Yasuo Ohgaki yohgaki@ohgaki.net:
It's been 2 weeks and no comment for this.
I'll start voting.
I'm sorry I have not caught this any earlier, but even without
consulting with Anatol, I'm not keen on adding this into 7.0, we are
in RC stage, and I want us to focus on a stable release instead of
adding more potential issues, I suggest you target 7.1 instead in the
RFC.
(Ferenc and Anatol cc'd)
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Hi Anatol and Ferenc,
I'm sorry I have not caught this any earlier, but even without
consulting with Anatol, I'm not keen on adding this into 7.0, we are
in RC stage, and I want us to focus on a stable release instead of
adding more potential issues, I suggest you target 7.1 instead in the
RFC.(Ferenc and Anatol cc'd)
I was expecting this kind of reply for this RFC.
What do you think?
This RFC contains JSON float value precision change for PHP 5.6 so that
PHP would not loose floating number information, as well as more precise
float handling option for PHP 7.0.
FYI, var_dump()'s float precision was changed to use serialize_precision as
bug fix before.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
-----Original Message-----
From: yohgaki@gmail.com [mailto:yohgaki@gmail.com] On Behalf Of Yasuo
Ohgaki
Sent: Sunday, September 20, 2015 11:49 PM
To: Kalle Sommer Nielsen kalle@php.net
Cc: Anatoliy Belsky ab@php.net; Ferenc Kovacs tyra3l@gmail.com;
internals@lists.php.net
Subject: Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float valueHi Anatol and Ferenc,
I'm sorry I have not caught this any earlier, but even without
consulting with Anatol, I'm not keen on adding this into 7.0, we are
in RC stage, and I want us to focus on a stable release instead of
adding more potential issues, I suggest you target 7.1 instead in the
RFC.(Ferenc and Anatol cc'd)
I was expecting this kind of reply for this RFC.
What do you think?This RFC contains JSON float value precision change for PHP 5.6 so that PHP
would not loose floating number information, as well as more precise float
handling option for PHP 7.0.FYI,
var_dump()'s float precision was changed to use serialize_precision as bug
fix before.
Thanks for the ping. IMHO this is the stuff for 7.1. I actually saw the discussions previously, but was rather thinking you was targeting 7.1 as that was already the time of the feature freeze.
Please keep in mind that we effectively have two RCs to do the remaining stabilizations. And we probably should as much as possible. Because this patch it's rather an enhancement to the current functionality which is not indeed broken - I wouldn't see it as critical for 7.0. It should be voted (and discussed if necessary) for 7.1, so we don't loop away from our current situation.
BTW I also had a comment there in the PR about https://github.com/php/php-src/pull/1455/files#diff-c84859666ff690a113d55ef1899d8bf4R149 - so if mode 0 ignores ndigits, why set it to 17 (or anything else). Seems like an unnecessary action, but snprintf is used quite frequently. But just as a notice on the side.
Regards
Anatol
Hi Anatol,
-----Original Message-----
From: yohgaki@gmail.com [mailto:yohgaki@gmail.com] On Behalf Of Yasuo
Ohgaki
Sent: Sunday, September 20, 2015 11:49 PM
To: Kalle Sommer Nielsen kalle@php.net
Cc: Anatoliy Belsky ab@php.net; Ferenc Kovacs tyra3l@gmail.com;
internals@lists.php.net
Subject: Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float valueHi Anatol and Ferenc,
I'm sorry I have not caught this any earlier, but even without
consulting with Anatol, I'm not keen on adding this into 7.0, we are
in RC stage, and I want us to focus on a stable release instead of
adding more potential issues, I suggest you target 7.1 instead in the
RFC.(Ferenc and Anatol cc'd)
I was expecting this kind of reply for this RFC.
What do you think?This RFC contains JSON float value precision change for PHP 5.6 so that PHP
would not loose floating number information, as well as more precise float
handling option for PHP 7.0.FYI,
var_dump()'s float precision was changed to use serialize_precision as bug
fix before.Thanks for the ping. IMHO this is the stuff for 7.1. I actually saw the discussions previously, but was rather thinking you was targeting 7.1 as that was already the time of the feature freeze.
No problem. I'll update so that 0 mode is for 7.1.
JSON's is better to use larger precision. So this change is targeted
to 5.6 and 7.0.
Let me know if you have comments on this.
Please keep in mind that we effectively have two RCs to do the remaining stabilizations. And we probably should as much as possible. Because this patch it's rather an enhancement to the current functionality which is not indeed broken - I wouldn't see it as critical for 7.0. It should be voted (and discussed if necessary) for 7.1, so we don't loop away from our current situation.
BTW I also had a comment there in the PR about https://github.com/php/php-src/pull/1455/files#diff-c84859666ff690a113d55ef1899d8bf4R149 - so if mode 0 ignores ndigits, why set it to 17 (or anything else). Seems like an unnecessary action, but snprintf is used quite frequently. But just as a notice on the side.
"-1" which is invalid precision is used to indicate 0 mode.
0 mode is supposed to use max precision for double (it seems many
implementations
use 16, but PHP uses 17), when 0 mode is used, it uses 17 as precision always.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
No problem. I'll update so that 0 mode is for 7.1.
JSON's is better to use larger precision. So this change is targeted
to 5.6 and 7.0.Let me know if you have comments on this.
The RFC is updated.
https://wiki.php.net/rfc/precise_float_value
Patches for 7.1/7.0/5.6 will be updated soon. 7.0/5.6's change will be
one liner.
i.e. Simply change to use PG(serialize_precision) instead of EG(precision).
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
Hi all,
No problem. I'll update so that 0 mode is for 7.1.
JSON's is better to use larger precision. So this change is targeted
to 5.6 and 7.0.Let me know if you have comments on this.
The RFC is updated.
https://wiki.php.net/rfc/precise_float_valuePatches for 7.1/7.0/5.6 will be updated soon. 7.0/5.6's change will be
one liner.
i.e. Simply change to use PG(serialize_precision) instead of EG(precision).
I don't really like that change in a point release. It will be a huge mess
and it's also quite a big BC break for point release in a function that is
used a lot on production systems. Let me remind you that it might change
considerably generated data size for some cases. It would be probably a
good idea to check with RM's if they are ok with such BC break before
voting.
What worries me even more is changing anything like this in 5.6 where json
ext is not used on the most used Linux distros (Debian based and RHEL based
ones) where json-c is used by default. It means that you would have to ask
Remi to change it too. If it was changed, then it becomes quite difficult
to find out what precision is actually used in which version. I think that
it would be just a huge mess... :).
Also there is no option for voters who would like to use
serialize_precision for json in 7.1. I would probably use one vote with
more options. Something like this:
JSON precision change
- no change
- use serialize_precision in 5.6
- use serialize_precicion in 7.0
- use serialize_precision in 7.1
- use json_precision in 7.1
The 5.6 and 7.0 should be there of course only if RM's are happy with this
BC break. I wouldn't personally allow that especially for 5.6. I think that
this change can wait till 7.1
In addition, the mode 0 voting is slightly unclear to me. It's one thing to
introduce mode 0 and another make it default. I'm actually for making it
default but it would be good to have that option in the voting. So how
about something like this?:
- do not introduce mode 0
- introduce mode 0 but keep current defaults
- introduce mode 0 and make it default
Cheers
Jakub
Hi Yasuo,
Thanks for the ping. IMHO this is the stuff for 7.1. I actually saw the
discussions previously, but was rather thinking you was targeting 7.1 as that was
already the time of the feature freeze.No problem. I'll update so that 0 mode is for 7.1.
JSON's is better to use larger precision. So this change is targeted to 5.6 and 7.0.Let me know if you have comments on this.
Thanks for retargeting the RFC.
BTW I also had a comment there in the PR about https://github.com/php/php-
src/pull/1455/files#diff-c84859666ff690a113d55ef1899d8bf4R149 - so if mode
0 ignores ndigits, why set it to 17 (or anything else). Seems like an unnecessary
action, but snprintf is used quite frequently. But just as a notice on the side."-1" which is invalid precision is used to indicate 0 mode.
0 mode is supposed to use max precision for double (it seems many
implementations use 16, but PHP uses 17), when 0 mode is used, it uses 17 as
precision always.
Exactly, so my question was - why it still needs to do "if (mode == 0) ndigit = 17;" in snprintf at the place I've linked? It won't have any effect as zend_dtoa will ignore it :)
Regards
Anatol
Hi Anatol,
On Sat, Sep 26, 2015 at 8:31 PM, Anatol Belski anatol.php@belski.net
wrote:
Hi Yasuo,
Thanks for the ping. IMHO this is the stuff for 7.1. I actually saw the
discussions previously, but was rather thinking you was targeting 7.1 as
that was
already the time of the feature freeze.No problem. I'll update so that 0 mode is for 7.1.
JSON's is better to use larger precision. So this change is targeted to
5.6 and 7.0.Let me know if you have comments on this.
Thanks for retargeting the RFC.
BTW I also had a comment there in the PR about
https://github.com/php/php-
src/pull/1455/files#diff-c84859666ff690a113d55ef1899d8bf4R149 - so if
mode
0 ignores ndigits, why set it to 17 (or anything else). Seems like an
unnecessary
action, but snprintf is used quite frequently. But just as a notice on
the side."-1" which is invalid precision is used to indicate 0 mode.
0 mode is supposed to use max precision for double (it seems many
implementations use 16, but PHP uses 17), when 0 mode is used, it uses
17 as
precision always.Exactly, so my question was - why it still needs to do "if (mode == 0)
ndigit = 17;" in snprintf at the place I've linked? It won't have any
effect as zend_dtoa will ignore it :)
As I said in the PR some time ago, it's not used by dtoa for mode 0 but
it's still used by php_gcvt for checking if exponential or decimal notation
should be used. See
https://github.com/php/php-src/blob/250938e2d35fc54161a18167b7901c5e3b574371/main/snprintf.c#L163
Cheers
Jakub
Hi Jakub,
-----Original Message-----
From: jakub.php@gmail.com [mailto:jakub.php@gmail.com] On Behalf Of Jakub
ZelenkaExactly, so my question was - why it still needs to do "if (mode == 0)
ndigit = 17;" in snprintf at the place I've linked? It won't have any
effect as zend_dtoa will ignore it :)As I said in the PR some time ago, it's not used by dtoa for mode 0 but it's still
used by php_gcvt for checking if exponential or decimal notation should be
used. Seehttps://github.com/php/php-
src/blob/250938e2d35fc54161a18167b7901c5e3b574371/main/snprintf.c#L163
Ah, I see it now. The patch is quite sparse, so have to check the whole functions. Thanks for the explanations.
For 7.0 raising the precision INIs to max supported were probably OK, but no new INI options should be added. And IMHO, there should be only one INI option change, and preferably full patch with json_precision in 7.1 and not touching other branches (the simplest solution). As it seems, one sees it critical to start using 17 digits in 7.0 - then probably vote were eligible (whereby I wouldn't see it critical). But then IMHO - switch to serialize_precision in 7.0 and keep the name for 7.1 with full patch. Though please be aware that times are going fast, if you decide to put something 7.0 related to the vote, be sure the results stand till RC5.
Regards
Anatol
Hi all and Ferenc,
-----Original Message-----
From: jakub.php@gmail.com [mailto:jakub.php@gmail.com] On Behalf Of Jakub
ZelenkaExactly, so my question was - why it still needs to do "if (mode == 0)
ndigit = 17;" in snprintf at the place I've linked? It won't have any
effect as zend_dtoa will ignore it :)As I said in the PR some time ago, it's not used by dtoa for mode 0 but it's still
used by php_gcvt for checking if exponential or decimal notation should be
used. Seehttps://github.com/php/php-
src/blob/250938e2d35fc54161a18167b7901c5e3b574371/main/snprintf.c#L163Ah, I see it now. The patch is quite sparse, so have to check the whole functions. Thanks for the explanations.
For 7.0 raising the precision INIs to max supported were probably OK, but no new INI options should be added. And IMHO, there should be only one INI option change, and preferably full patch with json_precision in 7.1 and not touching other branches (the simplest solution). As it seems, one sees it critical to start using 17 digits in 7.0 - then probably vote were eligible (whereby I wouldn't see it critical). But then IMHO - switch to serialize_precision in 7.0 and keep the name for 7.1 with full patch. Though please be aware that times are going fast, if you decide to put something 7.0 related to the vote, be sure the results stand till RC5.
Thank you for explanation, Jakub.
Ferenc, are you OK with the JSON serialize precision change?
i.e. Use PG(serialize_precision) rather than EG(precision) when
json_serialize() is called.
If yes, I'll prepare the patch and start the vote immediately.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless values.
This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!
Hi,
After asking Yasuo, I'm putting forward this RFC to have more precise float
to string decoding at least in PHP 7.1. I cleaned the RFC up a little bit
and it's targeting just 7.1.
It has been some time since this was announced so I will keep it open for a
week or two and then plan to start vote.
Cheers
Jakub
Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless values.
This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!Hi,
After asking Yasuo, I'm putting forward this RFC to have more precise float
to string decoding at least in PHP 7.1. I cleaned the RFC up a little bit
and it's targeting just 7.1.It has been some time since this was announced so I will keep it open for a
week or two and then plan to start vote.
Thanks for taking over this proposal!
I've already mentioned this on the PR when this was originally proposed,
but bringing it up here as well:
This proposal adds a new json.precision setting. Why? I've been told that
this is more flexible, which is fair enough, but imho we should have very
strong reasons for introducing new ini settings. Reasons that go beyond "it
might be useful to someone ... maybe?" So what's the particular use-case
here? Where is it necessary to export inaccurate floating point numbers in
JSON? And should such a use-case indeed exist, why is this a global setting
rather than an option of json_encode? Furthermore, note that even without
this new ini option, you always have the option of temporarily changing
serialize_precision for a json_encode call, if you really need it.
Please also take a look at this comment on the implementation:
https://github.com/php/php-src/pull/1455#discussion_r53933480
Regards,
Nikita
This proposal adds a new json.precision setting. Why? I've been told that
this is more flexible, which is fair enough, but imho we should have very
strong reasons for introducing new ini settings. Reasons that go beyond "it
might be useful to someone ... maybe?" So what's the particular use-case
here? Where is it necessary to export inaccurate floating point numbers in
JSON? And should such a use-case indeed exist, why is this a global setting
rather than an option of json_encode? Furthermore, note that even without
this new ini option, you always have the option of temporarily changing
serialize_precision for a json_encode call, if you really need it.
I second this remark especially because in my book JSON is just another
form of serialization, hence, it should use the same setting.
Additionally, how is it possible that global ini settings -- which in my
opinion is a sensitive area -- can go through with a 50%+1 vote?
--
Richard "Fleshgrinder" Fussenegger
This proposal adds a new json.precision setting. Why? I've been told that
this is more flexible, which is fair enough, but imho we should have very
strong reasons for introducing new ini settings. Reasons that go beyond
"it
might be useful to someone ... maybe?" So what's the particular use-case
here? Where is it necessary to export inaccurate floating point numbers
in
JSON? And should such a use-case indeed exist, why is this a global
setting
rather than an option of json_encode? Furthermore, note that even without
this new ini option, you always have the option of temporarily changing
serialize_precision for a json_encode call, if you really need it.I second this remark especially because in my book JSON is just another
form of serialization, hence, it should use the same setting.Additionally, how is it possible that global ini settings -- which in my
opinion is a sensitive area -- can go through with a 50%+1 vote?
Not exactly a language change as it's changing default just for
serialization and json but it's probably on the edge so will change it to
66 in the next revision... ;)
Cheers
Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless
values.This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!Hi,
After asking Yasuo, I'm putting forward this RFC to have more precise
float
to string decoding at least in PHP 7.1. I cleaned the RFC up a little bit
and it's targeting just 7.1.It has been some time since this was announced so I will keep it open for
a
week or two and then plan to start vote.Thanks for taking over this proposal!
I've already mentioned this on the PR when this was originally proposed,
but bringing it up here as well:This proposal adds a new json.precision setting. Why? I've been told that
this is more flexible, which is fair enough, but imho we should have very
strong reasons for introducing new ini settings. Reasons that go beyond "it
might be useful to someone ... maybe?" So what's the particular use-case
here? Where is it necessary to export inaccurate floating point numbers in
JSON? And should such a use-case indeed exist, why is this a global setting
rather than an option of json_encode? Furthermore, note that even without
this new ini option, you always have the option of temporarily changing
serialize_precision for a json_encode call, if you really need it.Please also take a look at this comment on the implementation:
https://github.com/php/php-src/pull/1455#discussion_r53933480
To be honest I have been thinking again about it before posting it. I
thought that it might be good to have at least a voting option or keep it
there for discussion but I don't really mind to drop it and just use just
serialize_precision by default. I know that it was me who suggested
json.precision initialy but it wasn't probably the best idea and it might
be a bit confusing for users. If no one thinks that it's useful and we
should have a vote about it, I will remove it in the next couple of days.
Cheers
Jakub
On Mon, May 30, 2016 at 7:28 PM, Nikita Popov nikita.ppv@gmail.com
wrote:Hi all,
IEEE 754 double cannot express exact float values. That said,
float values expressed by json/serialize/var_export/echo/print
are not precise enough in many cases.Issues:
json_encode()uses EG(precision)=14 that truncates float values.
echo()/print()
does this as well.- large EG(precision)/PG(serialize_precision) prints meaningless
values.This RFC proposes zend_dtoa()'s 0 mode support which rounds float value
to nearest value.https://wiki.php.net/rfc/precise_float_value
https://github.com/php/php-src/pull/1455This change is simple enough for PHP 7.0. IMO.
Comments/suggestions are appreciated!Hi,
After asking Yasuo, I'm putting forward this RFC to have more precise
float
to string decoding at least in PHP 7.1. I cleaned the RFC up a little bit
and it's targeting just 7.1.It has been some time since this was announced so I will keep it open
for a
week or two and then plan to start vote.Thanks for taking over this proposal!
I've already mentioned this on the PR when this was originally proposed,
but bringing it up here as well:This proposal adds a new json.precision setting. Why? I've been told that
this is more flexible, which is fair enough, but imho we should have very
strong reasons for introducing new ini settings. Reasons that go beyond "it
might be useful to someone ... maybe?" So what's the particular use-case
here? Where is it necessary to export inaccurate floating point numbers in
JSON? And should such a use-case indeed exist, why is this a global setting
rather than an option of json_encode? Furthermore, note that even without
this new ini option, you always have the option of temporarily changing
serialize_precision for a json_encode call, if you really need it.Please also take a look at this comment on the implementation:
https://github.com/php/php-src/pull/1455#discussion_r53933480To be honest I have been thinking again about it before posting it. I
thought that it might be good to have at least a voting option or keep it
there for discussion but I don't really mind to drop it and just use just
serialize_precision by default. I know that it was me who suggested
json.precision initialy but it wasn't probably the best idea and it might
be a bit confusing for users. If no one thinks that it's useful and we
should have a vote about it, I will remove it in the next couple of days.
I have updated the RFC and dropped the json.precision part. Instead of
that, there will be a voting option only whether to use serialize_precision
for json_encode.
If there are no other issues, I would like to open voting sometimes next
week.
Cheers
Jakub