fix(Unit): use the exact value for the acre unit#3665
Open
spokodev wants to merge 1 commit into
Open
Conversation
The acre was stored as a rounded `4046.86 m2`, while its defining units
`sqyd`, `sqft`, and `sqmi` are stored at full precision. An acre is defined
as exactly 4840 square yards, so it is `4046.8564224 m2`. Because of the
rounding, conversions were internally inconsistent:
math.evaluate('1 acre to sqyd') // was 4840.004..., now exactly 4840
math.evaluate('1 acre to sqft') // was 43560.04..., now exactly 43560
Use the exact value.
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
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.
Problem
The
acreunit is stored as a rounded4046.86 m2, while the units it is defined from are stored at full precision (sqyd: 0.83612736,sqft: 0.09290304,sqmi: 2589988.110336). An acre is exactly 4840 square yards =4046.8564224 m2, so the rounding makes conversions internally inconsistent:The relative error is ~8.8e-7 — far larger than IEEE-754 noise, since
4840 * 0.83612736is4046.8564224exactly.Fix
Store the exact value, matching the precision of the units the acre is defined from:
(The international acre = 4840 international square yards, consistent with mathjs's
sqyd/sqft/sqmi. The survey-basedsqrd/sqchare a separate question and are left unchanged.)Verification
Unit.test.jsasserting1 acre == 4840 sqyd == 43560 sqft; fails before, passes after.Unit.test.js: 130 passing, 0 regressions.1 acre to sqyd/to sqft/1 sqmi to acreare exact (relative error from 8.8e-7 to ~1e-16).