[RFC] [Discussion] Add 4 new rounding modes to round() function

Niels Dossche

2 years ago

Hey Jorg

Thanks for your RFC.

On 9/2/23 21:33, Jorg Sowa wrote:

> Hello everyone,
>
> I would like to start the RFC process about adding 4 new rounding modes to
> the round() function.
>
> As the topic was previously discussed and the change is quite small, I
> would like to start the voting on the beginning of next week.

I hate to be "that person", but I'm not sure you can do it that early.
I know you discussed the topic before (I believe that would count as the first step from https://wiki.php.net/rfc/howto).
As far as I know you have to wait at least 2 weeks to start the vote, counting from the moment you put the RFC in the "under discussion" phase.

>
> RFC: https://wiki.php.net/rfc/new_rounding_modes_to_round_function
>
> Thanks Niels for spotting incorrect links to the modes.
>
> Kind regards,
> Jorg
>

Kind regards
Niels

Tim Düsterhus

2 years ago

Hi

On 9/2/23 21:33, Jorg Sowa wrote:

> I would like to start the RFC process about adding 4 new rounding modes to
> the round() function.
>
> As the topic was previously discussed and the change is quite small, I
> would like to start the voting on the beginning of next week.
>
> RFC: https://wiki.php.net/rfc/new_rounding_modes_to_round_function
>

Is `round($num, 0, PHP_ROUND_CEILING) === ceil($num)` true for all
values of '$num' and likewise for `floor()`? If so, I think it would be
useful to spell this out explicitly in the RFC, because automated
refactoring tools might be interested in migrating to the unified
`round()` function.

Best regards
Tim Düsterhus

Tim Düsterhus

2 years ago

Hi

On 9/2/23 21:33, Jorg Sowa wrote:

> RFC: https://wiki.php.net/rfc/new_rounding_modes_to_round_function

The RFC mentions already has a section with regard to naming:

> Why not the names PHP_ROUND_UP and PHP_ROUND_DOWN

I generally agree with the argument in favor of the names you've chosen.
However they are inconsistent with the existing 'PHP_ROUND_HALF_UP' /
DOWN constants.

I don't want to create extra work for you, but I wonder if additionally
one of the following should be done:

- Creating PHP_ROUND_HALF_AWAY_FROM_ZERO as an alias for
PHP_ROUND_HALF_UP (and likewise for HALF_DOWN).
- Adding an entirely new \RoundingMode enum with clear names for all
variants. I feel like documenting such an enum with a dedicated page
would be easier compared to the PHP_ROUND_* constants which are not
directly related, except for the prefix.

Furthermore there will be another inconsistency by not having the
CEIL/FLOOR variants for the halfway point (i.e. half towards positive
infinity and half towards negative infinity).

Best regards
Tim Düsterhus

Alexandru Pătrănescu

2 years ago

Hi!

On Sun, Sep 3, 2023 at 3:14 PM Tim Düsterhus <tim@bastelstu.be> wrote:

>
> The RFC mentions already has a section with regard to naming:
>
>  > Why not the names PHP_ROUND_UP and PHP_ROUND_DOWN
>
> I generally agree with the argument in favor of the names you've chosen.
> However they are inconsistent with the existing 'PHP_ROUND_HALF_UP' /
> DOWN constants.
>
> I don't want to create extra work for you, but I wonder if additionally
> one of the following should be done:
>
> - Creating PHP_ROUND_HALF_AWAY_FROM_ZERO as an alias for
> PHP_ROUND_HALF_UP (and likewise for HALF_DOWN).
>
>

I believe consistency is important and using UP for "away from zero" and
DOWN for "towards zero" sounds good to me. That was so far used
consistently everywhere, as far as I'm aware.
Documentation is very clear about what they mean, and you can always think
that both positive infinity and negative infinity are up while zero is down.

Yes, we can have some extra constants for the four ones that contain "up"
or "down" with "away from zero" and "towards zero", but I find it
unnecessary.

Regards,
Alex

Tim Düsterhus

2 years ago

Hi

On 9/3/23 17:50, Alexandru Pătrănescu wrote:

> I believe consistency is important and using UP for "away from zero" and
> DOWN for "towards zero" sounds good to me. That was so far used
> consistently everywhere, as far as I'm aware.
> Documentation is very clear about what they mean, and you can always think
> that both positive infinity and negative infinity are up while zero is down.

I believe that the code should be easy to understand without needing to
consult the documentation. I don't find it obvious at all that "up"
would be the infinity with the same signum rather than the positive
infinity. This would likely be something I would need to look up all the
time.

In fact, let me quote from the Wikipedia article linked in the RFC:

https://en.wikipedia.org/wiki/Rounding#Rounding_half_up

> However, some programming languages (such as Java, Python) define

their half up as round half away from zero here.

Rounding "up" in Wikipedia is defined as rounding towards positive infinity:

https://en.wikipedia.org/wiki/Rounding#Rounding_up

> round up (or take the ceiling, or round toward positive infinity): y

is the smallest integer that is not less than x.

I'd also like to refer to this exchange of Mastodon regarding a very
similar issue with PHP 8.3's new IntervalBoundary enum:

https://phpc.social/@asgrim/109318512018298154

The terminology "closed" and "open" matches the standard terminology in
maths, but without the necessary background in maths it is not obvious
and indeed "inclusive" and "exclusive" might've been easier to
understand for everyone, including mathematicians [1].

In this case of rounding the existing behavior of "up" meaning "towards
the same-signum infinity" rather than "positive infinity" appears to
even contradict the definition used by Wikipedia which I also expect to
be the mathematical definition (but I'm happy to be corrected here).
"Away from zero" should be clear to everyone with a basic understanding
of English.

Introducing a brand-new RoundingMode enum (using union types) I
suggested in my first email would nicely solve the naming problem and
would also make the API more obvious to use due to stronger typing in
the long run.

> Yes, we can have some extra constants for the four ones that contain "up"
> or "down" with "away from zero" and "towards zero", but I find it
> unnecessary.

Best regards
Tim Düsterhus

[1] Back then I didn't have the energy to create a follow-up RFC and now
it's definitely too late. At least the naming is mathematically accurate.

Joerg Sowa

2 years ago

Thank you all for the comments. The naming brought some constructive
discussion.

My preferred solution would be inconsistent PHP_ROUND_AWAY_FROM_ZERO and
PHP_ROUND_TOWARD_ZERO as those are friendlier to the most of developers and
are not possible to mistake with other modes. I would go even one step
further and create aliases for NumberFormatter constants to have the naming
consistent.

> Introducing a brand-new RoundingMode enum (using union types) I
> suggested in my first email would nicely solve the naming problem and
> would also make the API more obvious to use due to stronger typing in
> the long run.

That's also interesting idea and I would push it, however I'm afraid about
the naming of namespace. I recollect some of the RFCs were rejected simply
because of the namespace. If there is a guideline for the namespace of core
functions I would willingly get familiar to it. Otherwise, I don't want to
open Pandora box only because of the namespace of Enum. It would also bring
BC to the function, unless we would accept both enum and old constants.

Kind regards,
Jorg

Tim Düsterhus

2 years ago

Hi

On 9/5/23 00:34, Jorg Sowa wrote:

>> Introducing a brand-new RoundingMode enum (using union types) I
>> suggested in my first email would nicely solve the naming problem and
>> would also make the API more obvious to use due to stronger typing in
>> the long run.
>
> That's also interesting idea and I would push it, however I'm afraid about
> the naming of namespace. I recollect some of the RFCs were rejected simply

I didn't intent to suggest putting the enum into a namespace. It would
be `\RoundingMode` (i.e. within the global namespace). I'm not sure how
you got the impression I suggest a namespace.

> because of the namespace. If there is a guideline for the namespace of core
> functions I would willingly get familiar to it. Otherwise, I don't want to

For the reference, there is a namespace guideline here:
https://wiki.php.net/rfc/namespaces_in_bundled_extensions

It doesn't really apply here, because `round()` is in ext/standard.

> open Pandora box only because of the namespace of Enum. It would also bring
> BC to the function, unless we would accept both enum and old constants.
>

Yes it would accept both the enum and the constants. In the long run the
constants could be deprecated. Alternatively the constants could be
aliases for the enum values, but this might break compatibility for
users that use the raw integers instead of the constants. While this is
"not a good idea", it might have happened.

Best regards
Tim Düsterhus

Jordan LeDoux

2 years ago

On Sun, Sep 3, 2023 at 5:51 AM Alexandru Pătrănescu <drealecs@gmail.com>
wrote:

> Hi!
>
> On Sun, Sep 3, 2023 at 3:14 PM Tim Düsterhus <tim@bastelstu.be> wrote:
>
> >
> > The RFC mentions already has a section with regard to naming:
> >
> >  > Why not the names PHP_ROUND_UP and PHP_ROUND_DOWN
> >
> > I generally agree with the argument in favor of the names you've chosen.
> > However they are inconsistent with the existing 'PHP_ROUND_HALF_UP' /
> > DOWN constants.
> >
> > I don't want to create extra work for you, but I wonder if additionally
> > one of the following should be done:
> >
> > - Creating PHP_ROUND_HALF_AWAY_FROM_ZERO as an alias for
> > PHP_ROUND_HALF_UP (and likewise for HALF_DOWN).
> >
> >
> I believe consistency is important and using UP for "away from zero" and
> DOWN for "towards zero" sounds good to me. That was so far used
> consistently everywhere, as far as I'm aware.
> Documentation is very clear about what they mean, and you can always think
> that both positive infinity and negative infinity are up while zero is
> down.
>
> Yes, we can have some extra constants for the four ones that contain "up"
> or "down" with "away from zero" and "towards zero", but I find it
> unnecessary.
>

As someone who maintains a math library in PHP that has its own expanded
rounding implementation, I will politely disagree. Away from zero, towards
zero, towards positive infinity, and towards negative infinity are
completely and utterly unambiguous and provide the developer with precise
control over what behavior they want. I don't see a reason to use something
that we KNOW is ambiguous when we're designing it.

This becomes even more important if we ever wanted to provide even more
rounding modes. For instance, a PHP_ROUND_HALF_ALTERNATING would want to
explicitly use positive and negative infinity as its directions most
likely. (The purpose of a "half alternating" mode is to ensure that the
introduced error by rounding many numbers doesn't have a directional bias
by swapping between two modes that round in one direction and then the
other.)

I don't want to complicate this RFC, but I wouldn't mind adding a
PHP_ROUND_STOCHASTIC mode later either. (This mode would round 1.7 to 2 for
70% of executions and 1 for 30% of executions. The purpose of this rounding
mode is to ensure that if you averaged the same number being rounded over
and over, it would average to your original unrounded number, and is used
in many Machine Learning applications.)

Jordan

Alexandru Pătrănescu

2 years ago

On Thu, Sep 7, 2023 at 11:10 PM Jordan LeDoux <jordan.ledoux@gmail.com>
wrote:

>
> As someone who maintains a math library in PHP that has its own expanded
> rounding implementation, I will politely disagree. Away from zero, towards
> zero, towards positive infinity, and towards negative infinity are
> completely and utterly unambiguous and provide the developer with precise
> control over what behavior they want. I don't see a reason to use something
> that we KNOW is ambiguous when we're designing it.
>
>

I agree. My main point was consistency.

On Tue, Sep 5, 2023 at 1:34 AM Jorg Sowa <jorg.sowa@gmail.com> wrote:

> Thank you all for the comments. The naming brought some constructive
> discussion.
>
> My preferred solution would be inconsistent PHP_ROUND_AWAY_FROM_ZERO and
> PHP_ROUND_TOWARD_ZERO as those are friendlier to the most of developers and
> are not possible to mistake with other modes. I would go even one step
> further and create aliases for NumberFormatter constants to have the naming
> consistent.
>
>

Yes, this is a great way to achieve consistency, adding aliases also in
NumberFormatter.

Thank you,
Alex

Joerg Sowa

2 years ago

Thank you all for the discussion. I modified description of the RFC and
added deprecation of constants ROUND_UP and ROUND_DOWN from Intl extension
as an additional voting.

Thanks also for input about possible mode ROUND_STOCHASTIC, however I think
it's the idea for the extension and not the addition to the standard
functions.

I would like to start the voting in coming days.

Kind regards,
Jorg

Girgias

2 years ago

On Wed, 4 Oct 2023 at 23:23, Jorg Sowa <jorg.sowa@gmail.com> wrote:

> Thank you all for the discussion. I modified description of the RFC and
> added deprecation of constants ROUND_UP and ROUND_DOWN from Intl extension
> as an additional voting.
>
> Thanks also for input about possible mode ROUND_STOCHASTIC, however I think
> it's the idea for the extension and not the addition to the standard
> functions.
>
> I would like to start the voting in coming days.
>

I don't think deprecating the constants in the version where the aliases
are introduced is a good idea.
There needs to be at least one version where both variants exist to allow
for cross version compatibility.

I will also echo Tim's idea of using an enum instead and a union type.
For the namespace, I think it would make sense to use Maths\RoundingMode

Best regards,

George P. Banyard

Joerg Sowa

2 years ago

> I don't think deprecating the constants in the version where the aliases

are introduced is a good idea.

I removed the deprecation. You are right.

> I will also echo Tim's idea of using an enum instead and a union type.
> For the namespace, I think it would make sense to use Maths\RoundingMode

Doesn't Math/Maths namespace introduce BC break?

Kind regards,
Jorg

Tim Düsterhus

2 years ago

Hi

On 10/5/23 23:37, Jorg Sowa wrote:

>> I will also echo Tim's idea of using an enum instead and a union type.
>> For the namespace, I think it would make sense to use Maths\RoundingMode
>
> Doesn't Math/Maths namespace introduce BC break?
>

Why do you think a namespace would introduce a BC break?

However I'd argue in favor of an enum, but within the global namespace.
i.e. just \RoundingMode.

https://github.com/search?q=%22class+RoundingMode%22+language%3Aphp&type=code

only finds namespaced versions, so this should not be a break.

If you prefer not to introduce an enum, then you should at least add
PHP_ROUND_HALF_AWAY_FROM_ZERO as an alias for
PHP_ROUND_HALF_UP (and likewise for HALF_DOWN) to make the constants
internally consistent.

Best regards
Tim Düsterhus

Joerg Sowa

2 years ago

Thank you for your comments. Listening to the opinions I will not include
deprecation of the constants and introducing the RoundingMode enum in this
RFC. Sorry for the delay. I will start eventually voting on the
RFC tomorrow.

Kind regards,
Jorg