Emit SymPy code that runs (#909) - #912
Merged
Merged
Conversation
MathS.ToSympyCode is documented as generating code you can run in SymPy, and for two whole
classes of expression it emitted code that did not run.
Rational's exporter was missing its closing parenthesis -- sympy.Rational(1, 2 -- so every
expression carrying a non-integer rational produced SyntaxError: '(' was never closed. That is
most of what a computer algebra system hands back.
Real emitted Stringize(), so the three non-finite values arrived as this library spells them:
NaN, +oo, -oo. The generated preamble binds a sympy.Symbol for each free variable, and none of
those three is a variable, so the program stopped with NameError. They now use SymPy's own
spellings, sympy.nan, sympy.oo and -sympy.oo.
Verified by running the emitted programs against SymPy 1.14 rather than by reading them, which
is also how it was established that they come back exact: 1/2 arrives as SymPy's Half and not as
the float 0.5. Eight expressions, previously two failures and six imprecise or broken, now zero
failures.
Nothing else changes. pi, e and i were already sympy.pi, sympy.E and sympy.I, and sqrt was
already sympy.sqrt -- measured before touching them, since the same-shape guess was wrong there.
The two new tests hold the properties that failed, without an interpreter in the suite: the
parentheses balance, and every name in the emitted body is either declared in the preamble or
reached through sympy. 17 of their 23 cases fail against the old exporter and 23 pass here.
ToSymPy is called only from ToSympyCode, so evaluation and simplification cannot see this;
suite 6474 passed, F# wrapper 130 passed, casbench 117/119 with 0 wrong.
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 #909, and it turned out to be worse than that issue described — the issue named the unbound names,
and the same file also had a missing parenthesis.
1/2,1/3 + 1/6sympy.Rational(1, 2—SyntaxError: '(' was never closedsympy.Rational(1, 2)0/0,1/0NaN—NameError: name 'NaN' is not definedsympy.nan+oo,-oo+oo,-oo—NameErrorsympy.oo,-sympy.ooThe parenthesis broke every expression carrying a non-integer rational, which is most of what a
computer algebra system hands back.
MathS.ToSympyCode's documented purpose is code you can run inSymPy, so this was not a cosmetic defect.
Measured by running it
SymPy 1.14 locally, eight expressions emitted and executed. Before: two
NameErrors and the rationalsrefusing to parse. After: zero failures, and the values come back exact rather than merely valid —
Half, not the float0.5, which is the whole reason to emitsympy.Rationalrather than a division oftwo Python integers.
What was already right
Checked before touching anything, because the "what else is the same shape" guess was wrong here:
pi,eandialready emittedsympy.pi,sympy.Eandsympy.I, andsqrtalready emittedsympy.sqrt.Only the two number cases above were broken.
The tests, without an interpreter in the suite
The suite cannot depend on Python, so the two properties that actually failed are what get asserted:
sympy.Rational(1, 2violated;sympy.Symbolin the preamble, or reachedthrough
sympy.— which is what a bareNaNor+ooviolated.The second generalises: any future value that leaves here as a bare name fails it. 17 of the 23 cases
fail against the old exporter, 23 of 23 pass here.
Not in this PR
1/2before simplification is aDivfof two integers and emits1 / 2, which Python evaluates to thefloat
0.5— exactness lost silently. Fixing it means deciding how much to sympify: emittingsympy.Integer(n)for every integer is faithful but makesx + sympy.Integer(1)ofx + 1. That is adesign choice rather than a defect fix, so it is filed separately as #911 with a recommendation.
ToSymPyis reached only fromToSympyCode, so evaluation and simplification cannot see any of this —verified by grep rather than assumed. Suite 6474 passed / 0 failed, F# wrapper 130 passed, casbench
117/119 with 0 wrong.
Cut from
masterat548ea178.