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

Bug: Incorrect encoding of complex64 numbers #1010

Closed
hemantjadon opened this issue Sep 10, 2021 · 1 comment · Fixed by #1011
Closed

Bug: Incorrect encoding of complex64 numbers #1010

hemantjadon opened this issue Sep 10, 2021 · 1 comment · Fixed by #1011

Comments

@hemantjadon
Copy link
Contributor

zap.Complex64 field types incorrectly formats the complex64 numbers. The precision (bitSize) information is lost while converting complex64.
Eg. 2.71+3.14i is formatted as 2.7100000381469727+3.140000104904175i

package main

import (
	"go.uber.org/zap"
)

func main() {
    l, _ := zap.NewProduction()

    l.Info("test", zap.Complex64("key", 2.71+3.14i))
}

Got output

{"level":"info","ts":1257894000,"caller":"sandbox265920417/prog.go:10","msg":"test","key":"2.7100000381469727+3.140000104904175i"}

Expected output

{"level":"info","ts":1257894000,"caller":"sandbox265920417/prog.go:10","msg":"test","key":"2.71+3.14i"}

Reference

https://play.golang.org/p/YH_NmnaRgBs

@hemantjadon
Copy link
Contributor Author

Reasoning behind this is similar to what was the case in #1002

abhinav added a commit that referenced this issue Sep 10, 2021
Per #1010, #1002, and #995, some of the corner cases where precision is
changed or lost aren't fully tested.

Add test cases for corner cases for a number of these:

- complex{64, 128}:  Test incorrect precision and negatives
- float{32, 64}: Test incorrect precision
- int{8, 16, 32, 64}: Test minimum and maximum values
- uint{8, 16, 32, 64}: Test maximum values

Per #1010, the test for complex64 incorrect precision is currently
failing.
abhinav added a commit that referenced this issue Sep 10, 2021
Converting `complex64` to `complex128` meant that
the real and imaginary components of the `complex64` value
were encoded as `float64` values which,
similar to #1002, encoded them with incorrect precision.

Fix this by adding a precision parameter during encoding to specify
the precision with which the components should be encoded.

Guard against other such cases where there's loss of precision
by adding tests for several such corner cases for
complex, float, int, and uint.

This has no significant performance impact on my laptop.
(The drop is likely a false positive.)

```
name                          old time/op  new time/op  delta
ZapJSONFloat32AndComplex64-4   491ns ±22%   442ns ± 6%  -10.00%  (p=0.000 n=9+9)
```

Fixes #1010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant