# [RFC] [DISCUSSION] More precise float value - Thread: https://externals.io/message/88042 - Original archive: https://news-web.php.net/php.internals/88042 - Started by Yasuo Ohgaki on 2015-09-04T00:41:04+00:00 - Messages: 19 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #88042 — Yasuo Ohgaki — 2015-09-04T00:41:04+00:00 URL: https://externals.io/message/88042#88042 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 --- ## #88359 — Yasuo Ohgaki — 2015-09-18T21:28:26+00:00 URL: https://externals.io/message/88042#88359 · In reply to: #88042 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! It's been 2 weeks and no comment for this. I'll start voting. Thanks. -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #88363 — Matt Wilmas — 2015-09-18T22:05:25+00:00 URL: https://externals.io/message/88042#88363 · In reply to: #88359 Hi Yasuo, ----- Original Message ----- From: "Yasuo Ohgaki" Sent: Friday, September 18, 2015 > Hi all, > > > > […] > > 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 --- ## #88364 — Kalle Sommer Nielsen — 2015-09-18T22:15:52+00:00 URL: https://externals.io/message/88042#88364 · In reply to: #88359 Hi Yasuo 2015-09-18 23:28 GMT+02:00 Yasuo Ohgaki : > 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 --- ## #88388 — Yasuo Ohgaki — 2015-09-20T21:48:48+00:00 URL: https://externals.io/message/88042#88388 · In reply to: #88364 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 --- ## #88390 — Anatol Belski — 2015-09-21T19:35:06+00:00 URL: https://externals.io/message/88042#88390 · In reply to: #88388 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 > Cc: Anatoliy Belsky ; Ferenc Kovacs ; > internals@lists.php.net > Subject: Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value > > Hi Anatol and Ferenc, > > > > […] > > 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 --- ## #88489 — Yasuo Ohgaki — 2015-09-24T22:49:58+00:00 URL: https://externals.io/message/88042#88489 · In reply to: #88390 Hi Anatol, > > […] > 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 --- ## #88490 — Yasuo Ohgaki — 2015-09-25T00:10:20+00:00 URL: https://externals.io/message/88042#88490 · In reply to: #88489 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 --- ## #88520 — Anatol Belski — 2015-09-26T19:31:58+00:00 URL: https://externals.io/message/88042#88520 · In reply to: #88489 Hi Yasuo, > > […] > 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. > > […] > 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 --- ## #88521 — Jakub Zelenka — 2015-09-27T17:22:54+00:00 URL: https://externals.io/message/88042#88521 · In reply to: #88490 Hi Yasuo, > Hi all, > > > > […] > > 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). > > 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 --- ## #88522 — Jakub Zelenka — 2015-09-27T17:29:21+00:00 URL: https://externals.io/message/88042#88522 · In reply to: #88520 Hi Anatol, On Sat, Sep 26, 2015 at 8:31 PM, Anatol Belski wrote: > Hi Yasuo, > > > […] > that was > > […] > 5.6 and 7.0. > > […] > Thanks for retargeting the RFC. > > > […] > https://github.com/php/php- > > […] > mode > > […] > unnecessary > > […] > the side. > > […] > 17 as > > […] > 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 --- ## #88523 — Anatol Belski — 2015-09-27T19:51:26+00:00 URL: https://externals.io/message/88042#88523 · In reply to: #88522 Hi Jakub, > -----Original Message----- > From: jakub.php@gmail.com [mailto:jakub.php@gmail.com] On Behalf Of Jakub > Zelenka > > […] > > 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 > 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 --- ## #88525 — Yasuo Ohgaki — 2015-09-28T08:28:27+00:00 URL: https://externals.io/message/88042#88525 · In reply to: #88523 Hi all and Ferenc, > > […] > 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. 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 --- ## #93634 — Jakub Zelenka — 2016-05-30T16:34:10+00:00 URL: https://externals.io/message/88042#93634 · In reply to: #88042 > 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! > > 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 --- ## #93635 — Nikita Popov — 2016-05-30T18:28:23+00:00 URL: https://externals.io/message/88042#93635 · In reply to: #93634 > > > > […] > 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 --- ## #93636 — Fleshgrinder — 2016-05-30T18:46:53+00:00 URL: https://externals.io/message/88042#93636 · In reply to: #93635 > 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 --- ## #93638 — Jakub Zelenka — 2016-05-30T19:01:53+00:00 URL: https://externals.io/message/88042#93638 · In reply to: #93635 > > > > […] > > 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 --- ## #93639 — Jakub Zelenka — 2016-05-30T19:07:04+00:00 URL: https://externals.io/message/88042#93639 · In reply to: #93636 > > > […] > "it > > […] > in > > […] > setting > > […] > > 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 --- ## #93820 — Jakub Zelenka — 2016-06-05T19:01:12+00:00 URL: https://externals.io/message/88042#93820 · In reply to: #93638 > On Mon, May 30, 2016 at 7:28 PM, Nikita Popov > wrote: > > > […] > 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. > > 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