Ian Lance Taylor uploaded time: use 1e9 rather than 1e-9 in Duration calculations for review.
time: use 1e9 rather than 1e-9 in Duration calculations 1e-9 has a 1 in the last place, causing some Duration calculations to have unnecessary rounding errors. 1e9 does not, so use that instead. Change-Id: I96334a2c47e7a014b532eb4b8a3ef9550e7ed057 --- M src/time/time.go M src/time/time_test.go 2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/time/time.go b/src/time/time.go index 03fde33..bee7e59 100644 --- a/src/time/time.go +++ b/src/time/time.go @@ -592,21 +592,21 @@ func (d Duration) Seconds() float64 { sec := d / Second nsec := d % Second - return float64(sec) + float64(nsec)*1e-9 + return float64(sec) + float64(nsec)/1e9 } // Minutes returns the duration as a floating point number of minutes. func (d Duration) Minutes() float64 { min := d / Minute nsec := d % Minute - return float64(min) + float64(nsec)*(1e-9/60) + return float64(min) + float64(nsec)/(60*1e9) } // Hours returns the duration as a floating point number of hours. func (d Duration) Hours() float64 { hour := d / Hour nsec := d % Hour - return float64(hour) + float64(nsec)*(1e-9/60/60) + return float64(hour) + float64(nsec)/(60*60*1e9) } // Add returns the time t+d. diff --git a/src/time/time_test.go b/src/time/time_test.go index 0af9da3..04ad546 100644 --- a/src/time/time_test.go +++ b/src/time/time_test.go @@ -1003,6 +1003,21 @@ } } +var secDurationTests = []struct { + d Duration + want float64 +}{ + {Duration(300000000), 0.3}, +} + +func TestDurationSeconds(t *testing.T) { + for _, tt := range secDurationTests { + if got := tt.d.Seconds(); got != tt.want { + t.Errorf("d.Seconds() = %g; want: %g", got, tt.want) + } + } +} + var minDurationTests = []struct { d Duration want float64 @@ -1011,6 +1026,7 @@ {Duration(-1), -1 / 60e9}, {Duration(1), 1 / 60e9}, {Duration(60000000000), 1}, + {Duration(3000), 5e-8}, } func TestDurationMinutes(t *testing.T) { @@ -1029,6 +1045,7 @@ {Duration(-1), -1 / 3600e9}, {Duration(1), 1 / 3600e9}, {Duration(3600000000000), 1}, + {Duration(36), 1e-11}, } func TestDurationHours(t *testing.T) { @@ -1117,8 +1134,8 @@ {"Truncate", func(t1, t2 Time) bool { return t1.Truncate(Hour).Equal(t2.Truncate(Hour)) }}, {"Round", func(t1, t2 Time) bool { return t1.Round(Hour).Equal(t2.Round(Hour)) }}, - - {"== Time{}", func(t1, t2 Time) bool { return (t1==Time{}) == (t2==Time{}) }}, + + {"== Time{}", func(t1, t2 Time) bool { return (t1 == Time{}) == (t2 == Time{}) }}, } func TestDefaultLoc(t *testing.T) {
To view, visit this change. To unsubscribe, visit settings.
Gobot Gobot posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.
Gobot Gobot posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.
Brad Fitzpatrick posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.
Ian Lance Taylor merged time: use 1e9 rather than 1e-9 in Duration calculations.
time: use 1e9 rather than 1e-9 in Duration calculations 1e-9 has a 1 in the last place, causing some Duration calculations to have unnecessary rounding errors. 1e9 does not, so use that instead. Change-Id: I96334a2c47e7a014b532eb4b8a3ef9550e7ed057 Reviewed-on: https://go-review.googlesource.com/33116 Run-TryBot: Ian Lance Taylor <ia...@golang.org> TryBot-Result: Gobot Gobot <go...@golang.org> Reviewed-by: Brad Fitzpatrick <brad...@golang.org> --- M src/time/time.go M src/time/time_test.go 2 files changed, 20 insertions(+), 3 deletions(-)
Approvals: Brad Fitzpatrick: Looks good to me, approved Ian Lance Taylor: Run TryBots Gobot Gobot: TryBots succeeded
diff --git a/src/time/time.go b/src/time/time.go index 175c9a9..00fafb6 100644 --- a/src/time/time.go +++ b/src/time/time.go @@ -603,21 +603,21 @@ func (d Duration) Seconds() float64 { sec := d / Second nsec := d % Second - return float64(sec) + float64(nsec)*1e-9 + return float64(sec) + float64(nsec)/1e9 } // Minutes returns the duration as a floating point number of minutes. func (d Duration) Minutes() float64 { min := d / Minute nsec := d % Minute - return float64(min) + float64(nsec)*(1e-9/60) + return float64(min) + float64(nsec)/(60*1e9) } // Hours returns the duration as a floating point number of hours. func (d Duration) Hours() float64 { hour := d / Hour nsec := d % Hour - return float64(hour) + float64(nsec)*(1e-9/60/60) + return float64(hour) + float64(nsec)/(60*60*1e9) } // Add returns the time t+d. diff --git a/src/time/time_test.go b/src/time/time_test.go index 07afcff..2922560 100644 --- a/src/time/time_test.go +++ b/src/time/time_test.go @@ -1003,6 +1003,21 @@ } } +var secDurationTests = []struct { + d Duration + want float64 +}{ + {Duration(300000000), 0.3}, +} + +func TestDurationSeconds(t *testing.T) { + for _, tt := range secDurationTests { + if got := tt.d.Seconds(); got != tt.want { + t.Errorf("d.Seconds() = %g; want: %g", got, tt.want) + } + } +} + var minDurationTests = []struct { d Duration want float64 @@ -1011,6 +1026,7 @@ {Duration(-1), -1 / 60e9}, {Duration(1), 1 / 60e9}, {Duration(60000000000), 1}, + {Duration(3000), 5e-8}, } func TestDurationMinutes(t *testing.T) { @@ -1029,6 +1045,7 @@ {Duration(-1), -1 / 3600e9}, {Duration(1), 1 / 3600e9}, {Duration(3600000000000), 1}, + {Duration(36), 1e-11}, } func TestDurationHours(t *testing.T) {
To view, visit this change. To unsubscribe, visit settings.
Rob Pike posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.
Side comments:
float64(1e9) is 0x41CDCD6500000000
which represents 1.86264514923095703125 * 2^29
aka, 1.00000000000000000000 * 10^9
float64(1e-9) is 0x3E112E0BE826D695
which represents 1.0737418240000000668743496134993620216846 * 2^-30
aka, 1.0000000000000000622815914577798564188970 * 10^-9
The difference observed, naturally, is the difference between “multiplies before divisions” (the numerical analysts mantra) and “avoid divides” (the cycle-counter’s mantra).
Since {1/(10^n), n integer > 0} is never exact in binary floating point, the issue is about the numerator size. If the numerator is big (a minute as 60 billion nanoseconds, for example) then the inexact representation of the 1e-9 will be hidden by the rounding after the multiply.
1/1.0000000000000000622815914577798564188970 * 10^-9 is
9.9999999999999993771840854222014746009961 * 10^8
that is, 10^9 - 6.22815914577798525399004*10^-8
so you have about 7 decimal places down there before you begin to see the problem with small numerators.
The real issue is not this instance, but the notion of using 1/x as an optimization; it works, but is a double-edged sword that cuts both ways.
Michael “The multiplies before divides instruction scheduler” Jones
--
You received this message because you are subscribed to the Google Groups "golang-codereviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-coderevi...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Martin Moehrmann posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.
Ian Lance Taylor posted comments on time: use 1e9 rather than 1e-9 in Duration calculations.
To view, visit this change. To unsubscribe, visit settings.