Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » rounding integers »

rounding integers

php.internals

Marc

3 years ago
Hi all, The "round" function supports rounding to significant digits before the decimal point which can be very helpful but unfortunately it's handling everything as floating point numbers which in the end can lead to inaccurate results.     printf("%d\n", round(987654321098765432, -1) // 987654321098765440 instead of 987654321098765430 To be fair it's also documented as such:
> round — Rounds a float
round(int|float $num, int $precision = 0, int $mode = PHP_ROUND_HALF_UP): float I would like to propose to change this function to keep the given type and proper handle integers as well:
> round — Rounds a number
/** @template T of int|float */ round(T $num, int $precision = 0, int $mode = PHP_ROUND_HALF_UP): T Do you think this could be an acceptable BC-break or should this be a different function? Thanks, Marc

Dan Ackroyd

3 years ago
On Sun, 21 May 2023 at 06:16, Marc <marc@mabe.berlin> wrote:
> > Do you think this could be an acceptable BC-break
No. Suggesting changing a 30 year old maths operations is a huge BC break.
> or should this be a different function?
Just make your own that does precisely what you want... cheers Dan Ack

Rowan Collins

3 years ago
On 21 May 2023 13:00:30 BST, Dan Ackroyd <Danack@basereality.com> wrote:
>On Sun, 21 May 2023 at 06:16, Marc <marc@mabe.berlin> wrote: >> >> Do you think this could be an acceptable BC-break > >No. Suggesting changing a 30 year old maths operations is a huge BC break. > >> or should this be a different function? > >Just make your own that does precisely what you want...
I agree on the first point, but disagree on the second. It's far too late to change round() itself, but the lack of built-in functions for converting to int in a controlled way is frustrating, particularly as the fashion moves towards stronger typing in general. On the surface, it sounds like a trivial operation, but there's a lot of edge cases to think about (limits of FP precision, negatives including negative zero, etc), and shipping a robust implementation of int_round, int_floor and int_ceil in core would save everyone having to rediscover them the hard way. Regards,
-- Rowan Tommins [IMSoP]

Larry Garfield

3 years ago
On Sun, May 21, 2023, at 7:18 AM, Rowan Tommins wrote:
> On 21 May 2023 13:00:30 BST, Dan Ackroyd <Danack@basereality.com> wrote: >>On Sun, 21 May 2023 at 06:16, Marc <marc@mabe.berlin> wrote: >>> >>> Do you think this could be an acceptable BC-break >> >>No. Suggesting changing a 30 year old maths operations is a huge BC break. >> >>> or should this be a different function? >> >>Just make your own that does precisely what you want... > > I agree on the first point, but disagree on the second. It's far too > late to change round() itself, but the lack of built-in functions for > converting to int in a controlled way is frustrating, particularly as > the fashion moves towards stronger typing in general. > > On the surface, it sounds like a trivial operation, but there's a lot > of edge cases to think about (limits of FP precision, negatives > including negative zero, etc), and shipping a robust implementation of > int_round, int_floor and int_ceil in core would save everyone having to > rediscover them the hard way.
Having recently been bitten by floor() returning a float even though the value that comes back is logically an int, I would be fully in support of "and returns an int" versions of these functions in core. --Larry Garfield

Marc

3 years ago
On 21.05.23 17:20, Larry Garfield wrote:
> On Sun, May 21, 2023, at 7:18 AM, Rowan Tommins wrote: >> On 21 May 2023 13:00:30 BST, Dan Ackroyd <Danack@basereality.com> wrote: >>> On Sun, 21 May 2023 at 06:16, Marc <marc@mabe.berlin> wrote: >>>> Do you think this could be an acceptable BC-break >>> No. Suggesting changing a 30 year old maths operations is a huge BC break. >>> >>>> or should this be a different function? >>> Just make your own that does precisely what you want... >> I agree on the first point, but disagree on the second. It's far too >> late to change round() itself, but the lack of built-in functions for >> converting to int in a controlled way is frustrating, particularly as >> the fashion moves towards stronger typing in general. >> >> On the surface, it sounds like a trivial operation, but there's a lot >> of edge cases to think about (limits of FP precision, negatives >> including negative zero, etc), and shipping a robust implementation of >> int_round, int_floor and int_ceil in core would save everyone having to >> rediscover them the hard way. > Having recently been bitten by floor() returning a float even though the value that comes back is logically an int, I would be fully in support of "and returns an int" versions of these functions in core.
Yes, ceil and floor have the same issue and probably some others as well. I remember number_format expecting float only. About changing the behavior of such an old method I also have mixed feelings about. On the one hand handling a number afterwards still wouldn't change but with more and more type safety it's probably breaking more then expected. A possible way could be to trigger some kind of warning if round/floor/ceil gets called with an integer but I don't see a good fit here as it's not a deprecation and notice/warning also does not make sense here I think. New function(s) on the other hand sounds like an ugly solution as well only helping people explicitly searching for it after they got bitten at least once. Scalar object methods, yes that would help in faaar future (only once until first release). Just thinking out loud. Marc

Robert Landers

3 years ago
> New function(s) on the other hand sounds like an ugly solution as well > only helping people explicitly searching for it after they got bitten at > least once.
What about deprecating round() in favor of round_float() and round_int()? or something. Robert Landers Software Engineer Utrecht NL

Marc

3 years ago
Hi again, On 22.05.23 13:55, Robert Landers wrote:
>> New function(s) on the other hand sounds like an ugly solution as well >> only helping people explicitly searching for it after they got bitten at >> least once. > What about deprecating round() in favor of round_float() and > round_int()? or something. > > Robert Landers > Software Engineer > Utrecht NL
I have done some research and worked on an implementation [1] even thought I'm not a C engineer. There are some things I have noticed: 1. From PHP's type system perspective an int is completely compatible to an int also in strict mode - https://3v4l.org/sspOM That means making round, ceil and floor returning an int should not be much less of an issue 2. The functions round, ceil and floor are documented to take an int|float as argument rather then a float only instead of a float only For me that means there is already some kind of difference expected as if it would be documented as float only 3. Documenting it as input type matches the output type does not work in corner cases as rounding up/down but integer numbers could lead to integer over/under flows which needs to be handled and casted into a floating point number falling back current behavior. 4. I figured out one more function "number_format()" which casts everything into a floating point number even though it can unnecessary ending up in rounding errors. Fixing this is not even a BC break at all and could go separately as a bug fix in my opinion. 5. It least in case of ceil and floor this would be a no-op instead of a cast + memory allocation for a new zval In case of round as well but there are more things to do - haven't done any performance tests so far I have the felling that this change is not that much of a BC break and serves the right purpose by reducing rounding errors as much as possible. Still as these are very highly used functions I would target the next major just as there BC breaks are somehow expected even though I now think the only BC break is on explicitly checking the types after rounding. Thanks, Marc [1] https://github.com/php/php-src/compare/master...marc-mabe:php-src:integer_rounding

David Gebler

3 years ago
On Sun, May 21, 2023 at 4:21 PM Larry Garfield <larry@garfieldtech.com> wrote:
> > Having recently been bitten by floor() returning a float even though the > value that comes back is logically an int, I would be fully in support of > "and returns an int" versions of these functions in core. >
What about adding a third, optional parameter to round() which is a flag (or bitmask) to specify preferred behaviour and return type? Kind of ugly, but maybe less ugly than another new function and no BC-break, as the default can be the existing behaviour. -Dave