Keep an exact value exact on the way to SymPy (#911) - #913
Merged
Conversation
The generated program ran and then quietly gave a different number. Python's / and its ** with a negative exponent are float operations on two integers, so 1 / 2 was 0.5 in the emitted code and sympify(0.5) is a Float rather than Rational(1, 2) -- the exactness gone before SymPy saw the expression, with nothing downstream able to recover it. 2 ** (-1) and 2 ** (-3) went the same way, as 0.5 and 0.125. Making one operand a SymPy integer hands the arithmetic to SymPy, which keeps it exact. sympy.Integer(1) / 2 is Rational(1, 2); sympy.Integer(2) ** (-3) is 1/8. Only a pair of integers is rewritten, which is the whole of what loses a value. With a symbol anywhere in the shape SymPy's operators already take over -- x / 2, 1 / x and x ** (-1) are untouched and were always exact -- and +, -, * and a non-negative ** are exact on Python integers, whose precision is unbounded, so 2 ** 70 was always right. Measured shape by shape rather than assumed, since the guess about which operators lose a value is the kind that is usually wrong. It bit only the unsimplified form, which is the one a caller writes: "1/2".ToEntity() is a Divf of two integers, because a printed rational parses back as a division (#873), while a simplified 1/2 is a Rational node and already emitted sympy.Rational(1, 2). Verified by running the emitted programs against SymPy 1.14, which is the only way this class of defect shows itself: the code was always valid Python, so #909's tests could not have caught it. Ten expressions, previously six of them inexact, now all exact and none failing. The two tests added here assert the property without an interpreter -- no two plain integer literals are combined with / or with a negative ** -- and the positive form for the shapes that must stay as they were. Suite 6488 passed, F# wrapper 130 passed, casbench 117/119 with 0 wrong. ToSymPy is reached only from ToSympyCode, so nothing in evaluation or simplification can see this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #911, taking that issue's second option — the narrow one, done in the two nodes where a value is
actually lost.
Where #909 was code that would not run, this is code that ran and then quietly gave a different
number. Python's
/, and its**with a negative exponent, are float operations on two integers:1/21 / 20.500000000000000, aFloatsympy.Integer(1) / 21/2, aRational2^(-1)2 ** (-1)0.5sympy.Integer(2) ** (-1)1/22^(-3)2 ** (-3)0.125sympy.Integer(2) ** (-3)1/8x + 1/2x + 1 / 2x + 0.5x + sympy.Integer(1) / 2x + 1/2Making one operand a SymPy integer hands the arithmetic to SymPy, which keeps it exact.
Only a pair of integers, and that is measured
The issue floated wrapping every integer, which is faithful but turns
x + 1intox + sympy.Integer(1). It isn't needed. I checked each operator shape against Python rather thanreasoning about it:
So two arms cover it:
Divf(Integer, Integer)andPowf(Integer, negative Integer). Everything else isbyte-for-byte what it was — with a symbol anywhere SymPy's own operators take over, and
+,-,*anda non-negative
**are exact on Python integers, whose precision is unbounded.It bit the form a caller writes
Only the unsimplified expression.
"1/2".ToEntity()is aDivfof two integers, because a printedrational parses back as a division (#873); a simplified
1/2is aRationalnode and already emittedsympy.Rational(1, 2). So the defect appeared exactly when someone exported what they wrote rather thanwhat the library computed, and gave no sign that anything had been lost.
Verified by running it
Against SymPy 1.14 locally — the only way this class shows itself, since the code was always valid
Python and so #909's tests could not have caught it. Ten expressions: six were inexact before, all ten
are exact now, none fails.
The two tests added here assert the property without an interpreter in the suite: no two plain integer
literals are combined with
/or with a negative**, plus the positive form for the shapes that muststay as they were. That is the strongest statement available without Python, and it generalises to any
future node that emits bare integer arithmetic.
Suite 6488 passed / 0 failed, F# wrapper 130 passed, casbench 117/119 with 0 wrong.
ToSymPyis reachedonly from
ToSympyCode, so evaluation and simplification cannot see this.Cut from
masterat32a90588.