golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.37k stars 17.71k forks source link

math: add Round #20100

Closed DavidR91 closed 7 years ago

DavidR91 commented 7 years ago

What version of Go are you using (go version)?

1.8

What did you expect to see?

A rounding method for floats in the standard library

What did you see instead?

There is no standard library method for rounding a float64 to int/int32

I would like to see the lack of this method perhaps reconsidered: I think I understand why it was originally omitted (lots of different methods can be used: implement it as per your requirements rather than expecting a silver bullet in the stdlib)

But, the result of this actually seems to have been that this method just gets banded around by copy-and-pasting. Lots of projects duplicate exactly the same method over-and-over, and many suggestions (e.g. stackoverflow answers) contain inaccurate or flawed rounding methods - some of which may well be making their way into production apps.

Personally, I think it would make sense for the stdlib to contain the most primitive/straightforward rounding method possible (e.g. the +/- 0.5 method as seen in Kubernetes) and if something more in-depth is required, it then becomes an exercise for the implementer

Original discussions:

Implement it in a library

An obvious counterargument is that rather than duplicating this method, just expose it in a community/third party library which projects can just consume.

You could certainly make a positive argument for establishing a full library of math routines that perhaps offers this functionality with a multitude of rounding modes (probably already exists?). My argument against this it that a third-party component should not be necessary for the most basic implementation of rounding: it should be the go-to for advanced mathematics requirements, not beginner-level math.

Additionally, the same arguments could be levelled at many core pieces of functionality inside Go itself (if you make this point, you have to consider how kind of absurd it is that zlib is deemed integral to the standard library, but implementing the most basic of rounding methods is not)

Overall

I would just really appreciate seeing this omission reconsidered. If it is a question of not wanting to decide which rounding method is included, I would very strongly suggest that just a trivial/basic implementation is added (if acceptable) so that this functionality is not constantly duplicated in projects

As per the above: if a user needs very specific requirements / rounding modes, that should be the jumping off point for implementing your own. But the simplest + dumbest possible rounding should be part of the language IMO

slrz commented 7 years ago

There is no standard library method for rounding a float64 to int/int32

No, it's built into the language instead and rounds towards zero. You're proposing to add another one that rounds half away from zero. Yet, you're saying that when the choice of rounding mode matters, you should probably go and implement your own.

DavidR91 commented 7 years ago

No, it's built into the language instead and rounds towards zero.

You're proposing to add another one that rounds half away from zero.

It's not really 'another one' at all. The fact you get floor by casting one type to another is an indirect result of truncation, I think it is quite a stretch to say it's built in rounding.

'Round half up' / away from zero is the intuitive concept of 'rounding' that most people expect/want 99% of the time.

cznic commented 7 years ago

'Round half up' / away from zero is the intuitive concept of 'rounding' that most people expect/want 99% of the time.

The only rounding need is trunc(f+0.5), ie. away from neg inf. I see nothing intuitive on changing the direction of the rounding for negative numbers.

Look at all the possibilities at Wikipedia.

DavidR91 commented 7 years ago

The only rounding need is trunc(f+0.5), ie. away from neg inf. I see nothing intuitive on changing the direction of the rounding for negative numbers.

I am aware of the various different rounding modes - and if you disagree with what I consider intuitive, that's fair.

But my point with this proposal was that I suspected the quantity of potential modes was the blocker for considering + implementing any round method in stdlib. I wanted to make it simpler to have it considered by limiting the scope to a very common + well understood mode (e.g. shared also by C99's roundf)

If the feedback is simply that lots of modes have to be supported in order for this proposal to be considered, that's fine - but obviously it is a significant amount of work to write + document and have unit tests for a number of different rounding modes (and therefore makes the proposal less likely to be considered)

cznic commented 7 years ago

I am aware of the various different rounding modes - and if you disagree with what I consider intuitive, that's fair.

FTR, in my post the crucial 'I' before 'need' was lost in edit, changing the intended meaning. I did not want to write it like it's the only needed rounding mode. Apologies.

dongweigogo commented 7 years ago

similar work in rust

ianlancetaylor commented 7 years ago

@dongweigogo From a quick look at the Rust work, that is equivalent to the existing Go functions math.Float64bits and math.Float64frombits. It's not related to the subject of this issue which is, essentially, math.Round.

I don't have any strong opinions on this issue. I note that, as mentioned above, C99 defines double round(double) which returns the nearest integer value of its argument, rounding halfway cases away from zero. It also defines long lround(double) which does the same only it returns an integer type rather than a floating point type. The existence of these functions in C99 is a good argument in favor of including them in Go's math package.

C99 also defines a current rounding mode, set by fesetround, and functions double nearbyint(double), double rint(double) and long lrint(double) that round to the nearest integer using the current rounding mode (the difference between nearbyint and rint is that the latter is permitted to raise the floating-point inexact exception). Implementations are not, however, required to support any particular rounding mode.

mpx commented 7 years ago

I think it's worth adding to math since creating the commonly desired rounding function "round half away from zero" (round(3)) appears obvious, but many straightforward implementations are likely to be subtly incorrect. Eg:

1) Not handling negative numbers and rounding in the "wrong" direction 2) When x = 0.5-ULP, Trunc(x+0.5) will incorrectly round to 1 (and similarly for negative numbers) 3) Not handling NaN correctly 4) When is x>=2^52, Trunc(x+0.5) may round to even before Trunc, effectively adding 1 [added 2017/06/07] 5) When -0.5 < x < 0, x may incorrectly round to 0 (instead of -0). [added 2017/09/02]

Hence the suggestion above using Trunc(f+0.5) for simple rounding fails due to (2), (4). https://play.golang.org/p/LNWIKWYlWe

I use the following as a "straightforward" implementation of round(3) in my code:

func Round(n float64) float64 {
    if n >= 0.5 {
        return math.Trunc(n + 0.5)
    }
    if n <= -0.5 {
        return math.Trunc(n - 0.5)
    }
    if math.IsNaN(n) {
        return math.NaN()
    }
    return 0
}

..but it's non-obvious enough that I wouldn't be surprised to find out this function is wrong too :). [2017/06/07, 2017/09/02] ..it is wrong due to (4), (5). CL proposed below is correct.

A round(3) equivalent in math should handle most uses of floating point rounding and prevent a lot of bugs. Maybe it is reasonable to leave other specific modes to the person who knows why they need them?

Also, a library implementation should be much more efficient using bit manipulation (but with an even less obvious implementation).

Azareal commented 7 years ago

I need one which rounds 0.17 to 0.2, 0.172 to 0.2, and so on. float64 rounding would need all sorts of varying precisions to be useful.

And if there's one for float64, then I think there should be one for float32 too. And sometimes, you just want to truncate a float, not round it. I have a control panel in one project where a load of information is presented to the end-user, but I don't want to annoy them with like 10 digits for every little thing.

btracey commented 7 years ago

@Azareal https://godoc.org/github.com/gonum/floats#Round

maddyblue commented 7 years ago

@btracey That function is also broken. See https://github.com/gonum/floats/blob/a2cbc5c70616cd18491ef2843231f6ce28b2cb02/floats.go#L605 where it returns if x*10^n > MaxFloat64, and returns the original x unchanged with no indication of error. This is more edge cases not correctly covered, due to a limitation in the implementation.

CockroachDB today realized that our round(float, n) implementation with RoundHalfEven rounding was wrong. Two of us spent half the day trying to figure out a correct implementation that didn't have edge cases. Our solution was to convert to an arbitrary-precision decimal, have it do the rounding (which is very well tested), then convert back to a float. This isn't fast, but at least it's correct. It is unfortunate that we couldn't figure out how to do this using just floats. (https://github.com/cockroachdb/cockroach/pull/15847)

It is unfortunate that Go programmers are spending so much time on this problem, and at best we can only produce broken implementations using existing float methods. Having a high quality rounding implementation would alleviate lots of duplicate and incorrect work.

btracey commented 7 years ago

Thanks for the report. Would you mind opening an issue?

rsc commented 7 years ago

It's certainly true that we don't want hidden state rounding modes. The C99 round family, as @ianlancetaylor points out, has a fixed rounding mode regardless of hidden mode. The only difference between round (returns float) and lround (returns long) is that the latter sets errno when it can't represent the result. We can probably let users check that and have plain

float64 Round(float64 f) // round to integer, halfway away from 0, like C round

If users need to convert to int, then instead of having an lround equivalent, they can use int(math.Round(f)) or int64(math.Round(f)) or whatever type is desired.

Accepting based on discussion with @golang/proposal-review.

rsc commented 7 years ago

For next round.

AlekSi commented 7 years ago

Accepting based on discussion with @golang/proposal-review.

Sorry for offtopic, but can you make this team visible somehow? For me, this link returns 404.

bradfitz commented 7 years ago

@AlekSi, it's currently @bradfitz, @rsc, @spf, @ianlancetaylor, @robpike, and @griesemer.

AlekSi commented 7 years ago

Making this team visible (https://github.com/orgs/golang/teams/proposal-review/edit) should fix 404 for other people, so you don't need to write this list down to README, Wiki, etc.

(Again, really sorry for derailing. If you don't totally agree with making this team visible, I will probably post to maillist to discuss it.)

cespare commented 7 years ago

I'd like to take this. Feel free to unassign me if you object.

btracey commented 7 years ago

Just to ask: Should the signature allow rounding to non-integers? For instance Round(v float64, prec int). This would enable @Azareal 's feature suggestion of being able to round 0.17234 to 0.172. I've used this behavior a few times, and implementing the full functionality would address the concerns of @mjibson about outside groups implementing the "precision" version with bugs.

griesemer commented 7 years ago

@btracey No. See also @rsc 's comment.

If you want to round to 3 digits after the comma, why can't you simply do Round(x * 1e3)/1e3?

btracey commented 7 years ago

Because of overflow (which is the error in Gonum's implementation as well).

griesemer commented 7 years ago

@btracey I assume you mean overflow of x * 1e3 in this case? How likely is this given the huge exponent ranges of floats?

btracey commented 7 years ago

I personally haven't come across the problem, but it sounds like @mjibson may have.

griesemer commented 7 years ago

I see.

Having a simple Round as proposed doesn't have this problem, of course. But if there's a chance that overflow can occur, then the number is so huge that rounding is pretty meaningless (the mantissa is not precise enough in the first place). Or I don't really understand the problem.

maddyblue commented 7 years ago

I care about the precision argument because it's part of an API I need to provide. I think that's an ok thing to not provide here as I have some special requirements, like round half even, which make more sense to implement on top of this round API.

gopherbot commented 7 years ago

CL https://golang.org/cl/43652 mentions this issue.

mpx commented 7 years ago

@cespare I wrote and tested an implementation the other day (now linked above), I hope you don't mind.

rsc commented 7 years ago

[off topic] @AlekSi, golang/proposal-review team is already "visible" but apparently (surprise to me) that does not mean publicly visible. I thought it was public; I don't know how to make it more public. But I confirm that it doesn't work when not logged in. The description says "A visible team can be seen and @mentioned by every member of this organization." I guess that means "not public". Ironically part of the reason I created that team list was to have a public list. Oh well, live and learn, and my apologies.

cespare commented 7 years ago

@mpx thanks!

DanieleDaccurso commented 7 years ago

Maybe not only having Round but also short-hand functions to Ceil / round-up and Floor / round-off would be nice.

hikari-no-yume commented 7 years ago

Might I point out that both x86/x64 and ARM can do IEEE 754-compliant rounding in a single instruction? It would seem wasteful not to make use of that where available.

ianlancetaylor commented 7 years ago

@hikari-no-yume Once we get the first CL, with tests, in for 1.10, please go ahead and send processor-specific optimizations as appropriate. Thanks.

dansouza commented 7 years ago

@hikari-no-yume came here to say this. I think we should just use the native instructions for float rounding by default on the default math.Round function (behaving as one would expect rounding to behave in their platform) and let the user decide the rounding mode and other peculiarities either by having different functions for each mode or passing an argument.

mpx commented 7 years ago

The implemention I've linked above is simple enough to be inlined by the compiler which makes it reasonably fast. Due to function call overhead, I suspect the native instructions may need to be an instrinsic to realise any significant benefit (but I haven't tested it).

a-h commented 7 years ago

I think it would be best to have a math/round package and call the Round function AwayFromZero in order to leave space for ToZero, ToEven, ToPositiveInfinity and ToNegativeInfinity variants.

Given that different programming languages have different implementations, having the rounding algorithm in the name usefully clarifies the exact implementation to the reader.

I also think it's important to be able to round to an arbitrary number of decimal places.

In my case, I needed this functionality when doing an online course where the tests needed to match an existing Python implementation exactly, so I wrote some rounding functions that did that and follows the naming pattern I outlined, but it doesn't handle some edge cases like infinity, NaN and overflow like a proper library implementation would.

mpx commented 7 years ago

Most of those rounding options already exist:

It's usually a mistake to round floating point to n decimal places where n > 0 since it may introduce an error (fractional base10 numbers can't be represented in base2). Depending on use case, it is usually better to either: 1) Round on output/display (fmt.Sprintf), or 2) Use a decimal library to perform the calculations in base 10.

a-h commented 7 years ago

I think you're right. I'm really only rounding to display and match other languages, so maybe I shouldn't care about having a rounding function which supports rounding to arbitrary digits.

I just expected it to be there based on other languages I use regularly like Python (https://docs.python.org/3/library/functions.html#round) and C# (https://msdn.microsoft.com/en-us/library/75ks3aby.aspx).

gdm85 commented 7 years ago

I made a gist for Round() for those who need it before February 2018 (planned release of Go 1.10)