fix: std.format %f uses round-half-away-from-zero instead of HALF_EVEN#1001
Open
He-Pin wants to merge 1 commit into
Open
fix: std.format %f uses round-half-away-from-zero instead of HALF_EVEN#1001He-Pin wants to merge 1 commit into
He-Pin wants to merge 1 commit into
Conversation
Motivation:
std.format("%.0f", [0.5]) returned "0" (banker's rounding / HALF_EVEN)
while go-jsonnet v0.22.0 and jrsonnet v0.5.0-pre99 both return "1"
(round-half-away-from-zero). The same HALF_EVEN divergence affected
every %.0f half-value such as 2.5 and -0.5.
Modification:
- DecimalFormat.scala precision==0 path: replace math.rint (HALF_EVEN)
with floor(x+0.5) / ceil(x-0.5) (half-away-from-zero).
- DecimalFormat.scala precision>0 path: apply BigDecimal.abs to the
scaled value so the sign is reconstructed from the input sign rather
than from the rounded magnitude. Existing tests did not hit a
negative-number failure in this path, but the arithmetic was unsafe
in principle and the change aligns the formula with the precision==0
intent.
- Add directional test format_rounding_half_away_from_zero.jsonnet
covering %.0f, %.1f, %.2f for positive/negative half values, the
carry case, and negative-to-zero rounding.
Result:
- std.format("%.0f", [0.5]) -> "1" (was "0")
- std.format("%.0f", [2.5]) -> "3" (was "2")
- std.format("%.0f", [-0.5]) -> "-1" (was "-0")
- std.format("%.0f", [-2.5]) -> "-3" (was "-2")
- %.1f and %.2f cases already matched go-jsonnet/jrsonnet on master
(0.25 and 0.005 have exact binary64 representations, so the previous
BigDecimal arithmetic happened to give the right answer). The
precision>0 change is a defensive cleanup, not a behavior fix.
- All existing tests continue to pass on JVM / JS / Wasm.
Cross-implementation verification (locally validated against go-jsonnet
v0.22.0 and jrsonnet v0.5.0-pre99 on macOS/arm64):
| Expression | go-jsonnet | jrsonnet | sjsonnet before | sjsonnet after |
|------------------------------|------------|----------|-----------------|----------------|
| std.format("%.0f", [0.5]) | "1" | "1" | "0" (broken) | "1" |
| std.format("%.0f", [2.5]) | "3" | "3" | "2" (broken) | "3" |
| std.format("%.0f", [-0.5]) | "-1" | "-1" | "-0" (broken) | "-1" |
| std.format("%.0f", [-2.5]) | "-3" | "-3" | "-2" (broken) | "-3" |
| std.format("%.1f", [0.25]) | "0.3" | "0.3" | "0.3" (ok) | "0.3" |
| std.format("%.1f", [-0.25]) | "-0.3" | "-0.3" | "-0.3" (ok) | "-0.3" |
| std.format("%.2f", [0.005]) | "0.01" | "0.01" | "0.01" (ok) | "0.01" |
| std.format("%.2f", [-0.005]) | "-0.01" | "-0.01" | "-0.01" (ok) | "-0.01" |
Tests:
- ./mill sjsonnet.jvm[2.13.18].test (all pass)
- ./mill sjsonnet.js[2.13.18].test (all pass)
- ./mill sjsonnet.wasm[2.13.18].test (all pass)
- scalafmt on changed files
References:
None — standalone bug fix.
He-Pin
force-pushed
the
worktree-fix-format-rounding-mode
branch
from
5831f20 to
b522792
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
std.format("%.0f", [0.5])returned"0"(banker's rounding /HALF_EVEN) while go-jsonnet v0.22.0 and jrsonnet v0.5.0-pre99 both
return
"1"(round-half-away-from-zero). The Jsonnet referencestandard library (
std.jsonnet) implements%frounding asstd.abs(n_) * denominator + 0.5followed bystd.floor, which ishalf-away-from-zero. sjsonnet's use of
math.rint(HALF_EVEN)diverged from this spec for every
%.0fhalf-value.Modification
DecimalFormat.scalaprecision==0 path: replacemath.rint(HALF_EVEN) with
floor(x+0.5)/ceil(x-0.5)(half-away-from-zero).
DecimalFormat.scalaprecision>0 path: applyBigDecimal.absto thescaled value and reconstruct the sign explicitly. This is a
defensive cleanup — all four callers in
Format.scalaalready passmath.abs(s), so the old code never saw negative inputs. The changemakes
DecimalFormatself-consistent and safe against future directcallers.
format_rounding_half_away_from_zero.jsonnetwith 16 assertions covering
%.0f,%.1f,%.2ffor positive andnegative half values, the carry case, and negative-to-zero rounding.
Result
Cross-implementation verification (validated locally against go-jsonnet
v0.22.0 and jrsonnet v0.5.0-pre99): all 16 test cases and 5 additional
edge cases produce identical output across all three implementations.
Only the
%.0fhalf-value cases were actually broken. The%.1f/%.2frows already matched becauseDecimalFormat.formatis alwayscalled with
math.abs(s)fromFormat.scala, so the old precision>0formula happened to give the right answer.
All existing tests continue to pass on JVM / JS / Wasm / Native / Graal.
References
std.jsonnet:numerator = std.abs(n_) * denominator + 0.5(half-away-from-zero algorithm in the reference standard library)