Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

math: add Round #20100

Closed
DavidR91 opened this issue Apr 24, 2017 · 38 comments
Closed

math: add Round #20100

DavidR91 opened this issue Apr 24, 2017 · 38 comments

Comments

@DavidR91
Copy link

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

@bradfitz bradfitz changed the title math: reconsider the lack of a standard library rounding function for float64 proposal: math: reconsider the lack of a standard library rounding function for float64 Apr 24, 2017
@gopherbot gopherbot added this to the Proposal milestone Apr 24, 2017
@slrz
Copy link

slrz commented Apr 25, 2017

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
Copy link
Author

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
Copy link
Contributor

cznic commented Apr 25, 2017

'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
Copy link
Author

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
Copy link
Contributor

cznic commented Apr 25, 2017

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
Copy link

dongweigogo commented Apr 27, 2017

similar work in rust

@ianlancetaylor
Copy link
Member

@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
Copy link
Contributor

mpx commented Apr 28, 2017

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
Copy link

Azareal commented May 5, 2017

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
Copy link
Contributor

btracey commented May 5, 2017

@maddyblue
Copy link
Contributor

maddyblue commented May 10, 2017

@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. (cockroachdb/cockroach#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
Copy link
Contributor

btracey commented May 10, 2017

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

@rsc
Copy link
Contributor

rsc commented May 15, 2017

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 rsc changed the title proposal: math: reconsider the lack of a standard library rounding function for float64 math: add Round May 15, 2017
@rsc rsc modified the milestones: Go1.10, Proposal May 15, 2017
@rsc
Copy link
Contributor

rsc commented May 15, 2017

For next round.

@AlekSi
Copy link
Contributor

AlekSi commented May 17, 2017

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
Copy link
Contributor

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

@AlekSi
Copy link
Contributor

AlekSi commented May 17, 2017

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
Copy link
Contributor

cespare commented May 17, 2017

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

@cespare cespare self-assigned this May 17, 2017
@btracey
Copy link
Contributor

btracey commented May 17, 2017

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
Copy link
Contributor

@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
Copy link
Contributor

btracey commented May 17, 2017

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

@griesemer
Copy link
Contributor

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

@btracey
Copy link
Contributor

btracey commented May 17, 2017

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

@griesemer
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@mpx
Copy link
Contributor

mpx commented May 18, 2017

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

@rsc
Copy link
Contributor

rsc commented May 18, 2017

[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 cespare removed their assignment May 18, 2017
@cespare
Copy link
Contributor

cespare commented May 18, 2017

@mpx thanks!

@DanieleDaccurso
Copy link

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

@hikari-no-yume
Copy link

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
Copy link
Member

@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
Copy link

dansouza commented Jul 10, 2017

@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
Copy link
Contributor

mpx commented Jul 13, 2017

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).

@golang golang deleted a comment from leoxiaoping Jul 20, 2017
@a-h
Copy link
Contributor

a-h commented Sep 1, 2017

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
Copy link
Contributor

mpx commented Sep 1, 2017

Most of those rounding options already exist:

  • math.Floor (toward -Inf)
  • math.Ceil (toward +Inf)
  • math.Trunc (toward 0)

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
Copy link
Contributor

a-h commented Sep 1, 2017

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
Copy link

gdm85 commented Nov 22, 2017

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests