Repository navigation
Compute xs:decimal arithmetic exactly - #688
Open
dbartholomae wants to merge 1 commit into
Open
dbartholomae wants to merge 1 commit into
dbartholomae wants to merge 1 commit into
Conversation
Two xs:decimals, or an xs:decimal and an xs:integer, are now added, subtracted, multiplied and divided on BigInt coefficients and rounded back to a number once, so 1.1 + 2.2 eq 3.3 holds. Values stay JavaScript numbers; double, float, untyped and integer arithmetic are unchanged. Operands of more than 15 significant digits keep number arithmetic. Makes K-NumericAdd-4, K-NumericMod-19, XMark-Q18 and XMark-All pass. Part of FontoXML#686 Co-authored-by: NeosiaNexus <63867369+NeosiaNexus@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Thanks for this. While the -47% slowdown for some functions scare me a bit, correctness is a whole lot more important to me than speed. And using FontoXPath for fast arithmetic is a bit of a stretch. @bwrrp would you have some time to check this out? Too bad https://github.com/tc39/proposal-decimal does not seem to be usable here (yet). |
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.
Part of #686
What changes
src/expressions/operators/arithmetic/decimalArithmetic.ts(new): exact+,-,*,div,idivandmodfor xs:decimal operands. Each operand is read as the shortest decimal its number prints as (String(0.1)is"0.1"), the operation runs on a BigInt coefficient with a decimal exponent, and only the result is rounded back to a number.1.1 + 2.2 eq 3.3is now true, and10.05 + 33.33is43.38rather than43.379999999999995. A quotient that does not terminate is carried to 20 digits plus a sticky digit before that final rounding, so it still rounds correctly.BinaryOperator.ts: whendetermineReturnTypegives xs:decimal (two decimals, or a decimal and an integer), the numeric branch takes the exact operation instead of the one in the operation map. The choice is made once per pair of operand types, where the map lookup already happens, so nothing is added per evaluation for other types.tsconfig.json,build.js:es2020.bigintadded tolib.targetstayses2017, so the code callsBigInt(10)rather than using10nliterals.test/assets/unrunnableTestCases.csv: the four QT3 tests that now pass are removed.What does not change
fontoxpath.d.ts, are byte-for-byte the same.@a + 0.2on an untyped attribute is still a double addition, as the spec requires.1.0 div 0.0still givesInfinity(the spec wants FOAR0001, which is a separate change).0.3333333333333333for1.0 div 3, and computing exactly on those digits is no more accurate than the float operation and sometimes one ulp further off.op-divide-dayTimeDuration-by-dTD-11regresses without this limit.2.0 * 3, number arithmetic is used, because the float operation is already exact for them.Tests
Unit tests:
BinaryOperator.tests.tsgains anxs:decimal arithmeticblock with 13 tests. They cover each operator, negative operands, non-terminating quotients, values cast from strings, a decimal meeting an integer, the string form of a result and the result type. Guards check that double, untyped and long operands keep their current results. Onmaster, the 8 that test exact results fail; the 5 that pin behaviour which must not change pass.QT3:
npm run qt3tests -- --regenerateK-NumericAdd-4,K-NumericMod-19,XMark-Q18andXMark-All(the last two convert currencies with2.20371 * $v). None newly fail.CI suites: all pass on Node 24 with the bundle built by
npm run build:npm run lintci-testtest-jscodegen, with and without--distintegrationtests -- --distqt3tests -- --distqt3testsxqueryx,xqutstests,xqutstestsxqueryxfuzzertestBundle size:
fontoxpath.jsgrows by 1,556 bytes (+0.5 %); gzipped by 595 bytes. Operators are looked up in aswitchrather than an object literal, because Closure's advanced mode renames the literal's keys and the lookup then misses in the bundle only.Benchmark: built bundles,
benchmarksuite, median of five alternating runs each, Node 24, Apple M1 Max:Integer, double, untyped and whole-number decimal arithmetic are within 2 %. Decimal arithmetic on fractional operands pays for the conversion: about 0.4 µs per operation here, which is 40–47 % in a loop that does nothing else and 17 % on a realistic expression. If that is too much,
+,-and*can stay in plain numbers whenever both scaled coefficients are below 2^53, with one division by a power of ten at the end, which is also correctly rounded; I kept this version simpler to review first.This builds on the idea in @NeosiaNexus's
fontoxpath-exact-decimalfork, credited as co-author. Maintainer edits are enabled.Disclosure: this pull request was written with an AI coding assistant (Claude Code). The tests, the QT3 runs, the benchmark and the bundle checks described above were run, not only written.