Skip to content

Round FormatFloat from the exact value - #173

Open
youdie006 wants to merge 1 commit into
dustin:masterfrom
youdie006:formatfloat-rounding
Open

youdie006 wants to merge 1 commit into
dustin:masterfrom
youdie006:formatfloat-rounding

Conversation

@youdie006

Copy link
Copy Markdown

FormatFloat adds 0.5 * 10^-precision and truncates, but that sum is itself rounded to a float, so the carry is often lost:

call master correct
FormatFloat("#,###.#", 8894.75) 8,894.7 8,894.8
FormatFloat("#,###.##", 71.575) (stored just above the tie) 71.57 71.58
FormatFloat("#,###.##", 945.125) (exact tie) 945.12 945.13, as 0.125 already gives 0.13

This lets strconv.FormatFloat round the binary value, and rounds exact ties up as the old rounder intended. On 150,000 random values and precisions 0-9, the output now matches Python's Decimal(x).quantize(..., ROUND_HALF_UP) in every case; master differs in 7,952. The new rows in TestFormatFloat fail on master. Plain strconv (ties to even) and treating any trailing 5 as a tie each fail at least one of them.

It costs about 200 ns per call instead of 110 on values without a tie (3 allocations instead of 1). A format with more than 9 decimals used to panic with an index out of range; it now formats. #167 moves the same lines, so whichever lands second will need a small rebase.

gofmt, go vet and go test ./... pass on Go 1.26; I did not run the 1.21 or 1.27 jobs locally.

Written with AI assistance (Claude); I have reviewed the change.

FormatFloat added 0.5 * 10^-precision and truncated, but the sum is
itself rounded to a float, so the carry was often lost: 8894.75 with
"#,###.#" gave 8,894.7 and 71.575 (just above the tie) gave 71.57,
while 0.125 rounded up and 945.125 rounded down. Let strconv round the
binary value, and round exact ties up as the old rounder meant to.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant