Add floor and ceil (#809) - #827
Merged
Merged
Conversation
Both were refused by name since #733. They exist now, as nodes rather than as sugar, with every convention measured against SymPy 1.14 rather than chosen -- AGENTS.md asks for exactly that when a convention has to be settled. What the measurement decided, and in three places it is not what the issue assumed: - Rounding is toward the infinities, not toward zero: floor(-3/2) is -2 and ceil(-3/2) is -1. - A complex argument is taken componentwise, so floor(3/2 + 5/2i) is 1 + 2i. SymPy and Mathematica agree, and it is the only reading under which floor of a real keeps its meaning when the imaginary part is zero. - The derivative is 0 provided the argument is not an integer. The issue guessed "zero almost everywhere"; SymPy declines to answer at all. Stating the condition says more than either, and it is what Signumf and Absf already do -- this library has Providedf and SymPy does not. ceiling( is accepted because that is SymPy's spelling, so an expression copied from there parses; Stringize prints the short ceil(, which is what the round trip pins. Inverting them is many-to-one: floor(f(x)) = v is solvable only for integer v and then f(x) is anywhere in [v, v + 1). So the inverse is a parameter over that interval, the same device Signumf and Absf use, and floor(x) - 3 = 0 now answers { 3 + t_1 provided t_1 in RR and t_1 >= 0 and t_1 < 1 } where #733 recorded it answering { 3 / floor }. The grammar was regenerated with JDK 25 and the committed ANTLR 4.13.1 jar. The unmodified grammar was regenerated first and reproduced the checked-in parser byte for byte, so the diff here is the new rules and not a toolchain difference. Seven tests in MissingFunctionNamesRefusedTest asserted these names were refused. That is the answer getting better rather than a test that was pinning a fudge: the refusal cases move to names still missing, and the equation from the original report is now asserted to be answered instead of refused. Tests: 5872 passing, 0 failed, 14 skipped; F# 130. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 9, 2026
Rafael-SOWNet
added a commit
that referenced
this pull request
Aug 9, 2026
* Bring Syntax.md up to date with the functions that now exist Syntax.md still listed floor, ceil, ceiling, round, min, max and gcd under "Refused by name". Six of them have existed since #827 and #828, so the document was telling a caller the opposite of what the parser does -- and AGENTS.md is explicit that a stale one of these is worse than none. I changed the grammar twice and did not update it either time. The entries say what each does rather than only that it parses, since the conventions are the part a caller cannot guess: rounding toward the infinities rather than zero, round going to the nearest even on a tie and so not being floor(x + 1/2), the componentwise reading of a complex argument, min and max leaving an unordered pair alone, and gcd covering rationals. Every claim in the new text was checked against a build rather than written from memory, including that the four still-refused names still raise and that re and im are still read as products. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say in Syntax.md too that LaTeX output is parsed elsewhere The same false claim #823 corrected in AGENTS.md -- "nothing parses LaTeX" -- was in Syntax.md as well, and I fixed only the one I had been pointed at. CSharpMath.Evaluation reads LaTeX back into an Entity and states the contract in its own source, so the sentence was wrong in both places. The point worth keeping is the one the correction adds: Latexise is still free to use \frac and the rest, but a change to what it emits can break a downstream project and nothing here will catch it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rafael-SOWNet
added a commit
that referenced
this pull request
Aug 9, 2026
… (#836) ComputeLimitDivideEtImpera defaulted to new Limitf(this, x, dist, side). That is the cycle AGENTS.md names, with this exact expression as its example: the caller evaluates the returned node to compare it, evaluating a Limitf computes the limit, and computing arrives back at the default. It overflows the stack, which kills the process rather than raising anything a caller can catch. So a node crashed by inheriting the default -- by not being mentioned anywhere. Seven did: limit(floor(x), x, 0) stack overflow, process dies limit(ceil(x), x, 0) stack overflow limit(round(x), x, 0) stack overflow limit(min(x, 1), x, 0) stack overflow limit(max(x, 1), x, 0) stack overflow limit(gcd(x, 2), x, 0) stack overflow limit(phi(x), x, 0) stack overflow The first six arrived with #827 and #828, which added the nodes without the ComputeLimitDivideEtImpera override that AddingNode.cs lists at step 3d. phi predates both and is #833, so this is not only my regression -- the default has been a landmine for every node that ever inherited it, which is what #704 was. Returning null is what AGENTS.md prescribes and what every caller already expects: each of the eleven call sites tests the result with `is { }` or switches on it, and the signature was already Entity?. All seven now come back as an unevaluated limit node, which is the honest "I could not settle this", and nothing else moves -- 5928 tests passed before this change and after it. LimitTerminatesOnEveryNodeTest enumerates the node types by reflection rather than listing them, because the defect is in what a node inherits by not being mentioned, and a per-node test cannot catch that: 45 types, each over three destinations and three sides, asserting only that the call returns. Checked against the unfixed code, where the run aborts. This does not replace #831, which gives floor and ceil real limit answers and fixes the OverflowException of #830. That adds capability; this removes the way to crash. They compose, and the order does not matter. Tests: 5973 passing, 0 failed, 14 skipped; F# 130. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rafael-SOWNet
added a commit
that referenced
this pull request
Aug 9, 2026
…#829, #830) (#831) * Answer the floor of an infinity instead of throwing (#830) floor(+oo), ceil(-oo) and floor(0/0) threw System.OverflowException out of evaluation. The Real branch converts through EInteger, which refuses both the infinities and NaN, and the exception reached the caller -- an internal PeterO.Numbers fault is not one a caller of Evaled has any reason to expect, and the neighbouring nodes do not raise it: abs(+oo) is +oo and abs(0/0) is NaN. An infinity is its own floor and its own ceil: there is no greatest integer below +oo, and every other system answers the infinity. NaN propagates, as it does everywhere else. Both are exact, so the new branch sits ahead of the !isExact guard and Simplify answers them too. eval(floor(+oo)) OverflowException -> +oo eval(ceil(-oo)) OverflowException -> -oo eval(floor(0/0)) OverflowException -> NaN Introduced by #827. Suite 5903 passed, casbench 113/117 with 0 wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Give floor and ceil a limit case instead of recursing (#829) A limit over floor or ceil overflowed the stack and killed the process. Neither node overrode ComputeLimitDivideEtImpera, so both took the base implementation, which returns an unevaluated limit of the very node being asked about. The two-sided path then compares its one-sided results by evaluating them, evaluating a limit computes it, and control arrives back at the same node. It is not an exception a caller can catch. This is #704 on a new pair of nodes. The Signumf override written for that issue carries a comment saying exactly this, and Modf's says it again for its own jumps; this adds the third case rather than a third copy of the reasoning. Away from its jumps a step function is locally constant, so the limit is the function of the argument's limit. On a jump the two sides disagree, and which side the argument arrives from is not decided by the side x approaches its destination from -- lim(x -> 0+) floor(2 - x^2) reaches 2 from below and lim(x -> 0+) floor(2 + x^2) from above, and both are limits from the right. So a jump is declined with null, which hands the caller an unevaluated limit, and no value is invented for it. An infinity is not a jump: it is its own floor, which is what #830 made answerable, so the limits at the infinities come out rather than being declined. lim(x -> 0) floor(x) *** stack overflow *** -> limit(floor(x), x, 0) lim(x -> 2) floor(x) *** stack overflow *** -> limit(floor(x), x, 2) lim(x -> 1/2) floor(x) *** stack overflow *** -> 0 lim(x -> +oo) floor(x) *** stack overflow *** -> +oo lim(x -> -oo) ceil(x) *** stack overflow *** -> -oo The imaginary part is only checked for a genuinely complex limit: a real value's imaginary part is identically zero rather than tending to zero, and the floor of a constant zero is constant. Like the Absf and Signumf overrides beside it, this reads the argument as real-valued along the path; narrowing that wants a way to decide realness the library does not have yet (#721), and it is noted in the remarks rather than assumed silently. Introduced by #827. Suite 5903 passed, F# 130, casbench 113/117 with 0 wrong, rootcheck 596/596, simpsweep 10463/10463, propcheck 1340 checks 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- 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.
Part of #809 — the two least contested of the six functions it asks for.
round,min,max,gcdandlcmare untouched and still refused by name.Every convention was measured, not chosen
AGENTS.md asks for this when a convention has to be settled, so the values come from SymPy 1.14 rather than from reasoning. Three of them are not what the issue assumed:
floor(-3/2)is-2,ceil(-3/2)is-1floor(3/2 + 5/2i)is1 + 2i. SymPy and Mathematica agree, and it is the only reading under whichfloorof a real keeps its meaning when the imaginary part happens to be zero0 provided not x in ZZ. The issue guessed "zero almost everywhere"; SymPy declines to answer at all. Stating the condition says more than either, and it is exactly whatSignumfandAbsfalready do — this library hasProvidedfand SymPy does notceiling(is accepted too, since that is SymPy's spelling and an expression copied from there should parse.Stringizeprints the shortceil(, which is what the round-trip test pins.Inverting them
floor(f(x)) = vis solvable only wherevis an integer, and thenf(x)is anywhere in the half-open interval[v, v + 1)— so the inverse is a parameter over that interval, not a point. That is the same deviceSignumfandAbsfuse for their own many-to-one inverses.#733 recorded that same equation answering
{ 3 / floor }— a root of nothing. It is now answered correctly, and there is a test asserting it.The grammar
Regenerated with JDK 25 and the committed ANTLR 4.13.1 jar, followed by the post-processing step. The unmodified grammar was regenerated first and reproduced the checked-in parser byte for byte, so the diff here is the new rules and not a toolchain difference — which is the check
Docs/Contributing/ImproveParser.mdasks for and the only reason this was safe to touch.Three lines moved out of the not-implemented list and into the implemented one.
Tests
5872 passing, 0 failed, 14 skipped. F# 130 passing.
FloorCeilTestcovers values against SymPy's answers (including every negative case), the complex componentwise rule, idempotence, symbolic arguments left alone, the derivative, LaTeX, the printed form parsing back, substitution, and the solve.Seven existing tests failed and were changed, and it is worth being explicit about which kind that is: they asserted
floor/ceil/ceilingare refused, and they no longer are. That is the answer getting better, not a test pinning a fudge. The refusal cases move to names that are still missing, and one gains a counterpart asserting the original report's equation is now answered. A new test covers the neighbouring case — an implemented name with the wrong argument count now says the count is wrong rather than that the name is unknown.Compatibility
BREAKING-CHANGES.mdrecords it: these three spellings used to raiseUnrecognizedFunctionParseExceptionand now return expressions. If you were catching that around these names, it is dead code.floorwithout a bracket is still an ordinary variable, and the other refused names are unchanged.Not in this change
roundneeds the tie-breaking convention writing down — measured as half-to-even in SymPy, Python and IEEE 754, but not whatMath.Round(decimal)does, so it deserves its own note.min/maxwant the node-versus-sugar decision, on which SymPy keeping them as nodes is evidence.gcdturned out to be the polynomial gcd rather than an integer one, and this library already hasPolynomialGcd— so it should be looked at alongside that rather than as new number theory. All three are recorded on #809 with the measurements.🤖 Generated with Claude Code