Repository navigation
Stringize of a Rational parses back as a Divf, so the round trip is not an identity #873
Description
Stringize is meant to produce something that parses back to the same expression — 2.0 fixed several node shapes that did not, and EveryNodeSurvivesEveryPipelineTest holds the tree to it. A Rational still does not.
Entity original = "3.5".ToEntity(); // Rational, 7/2
var printed = original.Stringize(); // "7/2"
Entity back = printed.ToEntity(); // Divf(7, 2)
original == back; // falseMeasured on master at 07a28c2:
| input | prints as | comes back as | equal |
|---|---|---|---|
3.5 |
7/2 |
Divf |
no |
0.1 |
1/10 |
Divf |
no |
2.75 |
11/4 |
Divf |
no |
1/2 |
1 / 2 |
Divf |
yes — it was a Divf to begin with |
The value survives; the node does not. Entity equality is structural, so the two are unequal, and anything that relies on the round trip being an identity is quietly wrong.
Why it matters beyond tidiness
I hit this implementing Serialize/Deserialize for matrix elements (two of the twelve gaps behind the old FutureReleaseException, see #872). Printing to text and parsing back is the natural implementation, since it needs no second format to keep in step with the printer — except that it silently changes 3.5 into a division node, so a matrix would not survive its own round trip. That work is on hold rather than shipped.
Why it is awkward to fix
There is no rational literal in the grammar. 7/2 is a division, so a Rational has no printed form that parses straight back to a Rational.
The obvious patch — deserialize through InnerSimplified, collapsing Divf(7, 2) to Rational(7/2) — breaks the other direction: an expression that genuinely was Divf(1, 2), from a caller writing 1/2, would come back as a Rational instead. So it trades one asymmetry for its mirror image.
Possible directions, none obviously right:
- Accept it and document that
Stringizepreserves value, not node identity. Then anything wanting a faithful round trip needs a real serializer rather than the printer. - Give the grammar a rational literal, so
Rationalhas a printed form of its own. - Normalise at the boundary: decide that
Divfover two integer literals is aRationalafter parsing, making the two indistinguishable rather than reconciled. That is a parser change with a wide blast radius.
Filing rather than picking, since which of those is right depends on what Stringize's contract is meant to be, and that is a decision rather than a defect.
Separately
integral(x ^ 2, x, 1) throws FunctionArgumentCountException on parse, so it does not round-trip either — but that is #847 and already known.
Activity
Happypig375 commentedon Aug 11, 2026
if you have two integers on both sides of the division, it is reasonable to try to parse it as a rational
Keep an exact value exact on the way to SymPy (#911) (#913)
Rafael-SOWNet commentedon Aug 15, 2026
Re-measured on
masterat2d128f3c, since the table above was taken on07a28c2b. Still true, and the same four rows still fail.@Happypig375's rule — two integers either side of a division parses as a rational — settles the question this was filed to ask. Three things I found while checking it that bear on doing it.
1. The parser would only be catching up with the normalisation.
Divf(1, 2).InnerSimplifiedis already aRationaltoday. So the objection in the issue body — that collapsingDivf(7, 2)would wrongly turn a caller's1/2into aRational— describes something the pipeline already does one step later. Parsing it as a rational does not introduce a new identification; it makes the parser agree withInnerSimplifiedinstead of disagreeing with it for one pass.2. The blast radius is smaller than the issue claims, and mostly negative. Eleven test sites mention
Divf; most are unrelated (cost model, pattern tests, GCD). Three exist only because of this bug and would get simpler:EveryNodeSurvivesEveryPipelineTest.KnownRoundTripFailurescarries["1/2"]as a known exclusionToSympyCodeTesttakes its cases unsimplified and says "which is Stringize of a Rational parses back as a Divf, so the round trip is not an identity #873"SimplificationRegressionTestsimplifies an expectation because"1/2"parses as aDivfThe visible change is printing: a parsed
1/2would print1/2rather than1 / 2, sinceRationalandDivfspace differently. That is aBREAKING-CHANGES.mdentry, not a blocker.3.
Complexhas the identical defect, and this rule does not fix it. The exclusion list has two entries, not one, and the second is the same shape:Rational7/27/2DivfRational-7/2-7/2DivfComplex1+2i1 + 2iSumfComplex3-4i3 - 4iMinusfA
Complexwith a non-zero real part prints as a sum and parses back asSumf— orMinusfwhen the imaginary part is negative, which is a third shape to recognise. Note"1 + 2i"typed by a caller has always been aSumf, so only a constructedComplexreaches this; that is exactly the serializer case in the issue body, since a matrix element is constructed rather than parsed.So the parallel rule is "a sum or difference of a real literal and an imaginary one is a
Complex". Worth deciding now whether it is in scope: fixing only the rational shrinksKnownRoundTripFailuresfrom two entries to one, and leaves the round trip still not an identity for the case that motivated the issue.Happy to implement whichever scope you want — rational only, or both.
Rafael-SOWNet commentedon Aug 16, 2026
Implementation attempt, and why I am not putting up a PR yet: the parser-side fix is blocked behind a
Replacedefect, now #955.I built the rational half — a quotient of two integer literals becomes the
Rationalit denotes, applied to the parsed tree, with a quotient that reduces to an integer deliberately left alone so that4/2stays aDivfrather than the parser evaluating it. The round trip becomes an identity for every case in the table above:3.57/2Rational0.11/10Rational1/21/2Rational4/24 / 2Divf1/01 / 0DivfThen the suite found ten failures, and two of them were not test-expectation churn:
domain(1/2, ZZ)stopped answeringNaNand started answering1/2, anddomain(sqrt(4/9), ZZ)likewise.domain(...)does not wrap a node, it attaches aCodomainto it — and rewriting the tree withEntity.Replacerebuilds every node on the path to the change, dropping the annotation. That is #955, and it reproduces onmasterwith no change of mine.I fixed the immediate node's codomain and the ancestor case still failed, which is what sent me looking. So this is not something #873 can work around: any post-parse rewrite has the same hole, and a version that quietly weakened domain constraints would be trading a loud failure — a round trip that visibly is not an identity — for a silent one, which is the worse of the two.
So the order is #955 first, then this. The other eight failures are ordinary and expected: six
TestDivideByEntityStrictrows whose expectation strings now parse asRationalwhile substitution still produces aDivf, oneToSympyCodeTestrow that exists only to document this bug, and theEveryNodeSurvivesEveryPipelineTestexclusion entry, which the fix deletes.Branch kept locally rather than pushed. Still happy to do the
Complexhalf too once the scope question above is answered.Keep a node's codomain when a rewrite rebuilds it (#955) (#956)
Rework the six expectations that can no longer be written as text (#873)
Parse a quotient of two integer literals as the Rational it denotes (#…
Rafael-SOWNet commentedon Aug 23, 2026
Fixed, and not by anything aimed at this issue. Measured on
masterat3ac24bc2:Entity equality holds across the trip in every case, and it holds for the shape that matters most here — a
Rationalthat arithmetic produced rather than the parser:The cause was the parser rather than
Stringize: a quotient of two integer literals used to parse as aDivf, so printing aRationalproduced text that read back as a division. That was fixed independently and is recorded inBREAKING-CHANGES.mdas ""7/2".ToEntity()and any quotient of two integer literals — was aDivf, is theRationalit denotes". This issue closed with it.Found while re-running the workspace harnesses after tonight's merges; flagging it rather than assuming it, since a recorded verdict here has been stale before.
Rafael-SOWNet commentedon Aug 23, 2026
Re-measured on
masterat6b93b401(2.3.0), .NET 10, default settings. Every row of the table above now survives, compared as entities rather than as printed text:3.57/2Rational0.11/10Rational2.7511/4Rational1/21/2RationalFixed by #957, which took the third of the three directions this issue set out — "normalise at the boundary: decide that
Divfover two integer literals is aRationalafter parsing, making the two indistinguishable rather than reconciled". So the mirror-image asymmetry the issue worried about does not arise: a caller who writes1/2and a caller who writes0.5now get the same node, which is what makes the round trip an identity rather than a coincidence.That also unblocks what the issue said it was blocking — printing to text and parsing back is now a faithful round trip for rationals, so a matrix survives its own.
Closing on the measurement. The residual question the issue raised in passing — whether
Stringize's contract is about value or about node identity — is now answered by the code rather than left open: it is node identity, andEveryNodeSurvivesEveryPipelineTestholds it there.Print a node's Codomain, so the printed form stops losing it (#1022)
Print a node's Codomain, so the printed form stops losing it (#1022) (#…
An annotated rational literal keeps its annotation, CC included (#1120)