Bring the simplification contract up to date, and measure the rule it still names - #917
Merged
Merged
Conversation
… still names Three things #916 made stale: the DomainCondition table still gave the logarithm's real condition, section 7's 'ln and sqrt disagree about which reading their domain describes' was fixed by it, and section 8 said the rules-driven harness 'does not exist yet' when boundcheck is that harness and was built in the same PR that wrote this file. Section 11 is new, and is a measurement rather than a plan. ln(a) + ln(b) -> ln(a*b) is boundcheck's last disagreement and carries no condition, so by O2 it asserts there is nothing to assume, which is false. Guarding it with the file's own IsPositiveReal idiom fails three tests and then hangs -- three and a half hours against a normal five minutes, with four of the five candidate tests under --blame-hang being limit tests. So the dependency is termination, not coverage: the limit machinery expands logarithms and relies on the simplifier to gather them back. TRIAGE recorded the cost as 'the log-equation solver loses coverage', and that is not what it is. The precedent is eight rules up in the same file -- a^n / b^n was guarded for this reason and repaired by teaching the limit reader to recognise the shape itself, where IsEventuallyPositive can check positivity on the approach. That repair also needed a re-read bound, which the logarithm path has no equivalent of, and which is the likeliest reason its symptom is a hang. Section 10 gains the caution that follows: this rule was the standing example of what per-symbol assumptions would rescue, and no assumption on a and b discharges a termination dependency. Issue: #721 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.
Documentation only. Two halves: what #916 made stale, and a measurement of the one rule
boundcheckstill reports.Stale after #916
DomainConditiontable still gave the logarithm's real condition (ln(x)→x > 0). It is now the complex one, with a note thatWithCodomain(Domain.Real)gives the other and that which you get is the reading.lnandsqrtdisagree about which reading their domain describes" — was fixed by Let the logarithm's domain follow the reading (#721, #890) #916. Struck, with the reason it is worth remembering: the condition cannot be read off evaluation, becauseln(0)is-ooand-oois not a complex number.boundcheckis that harness and was built in the same PR that wrote this file.§11 — new, and a measurement rather than a plan
ln(a) + ln(b) -> ln(a*b)isboundcheck's last disagreement: atx = -3,ln(x) + ln(x+1)is1.7918 + 6.2832iandln(x*(1+x))is1.7918, differing by the2*pi*ithe principal branch discards. The rule carries no condition, so by O2 it asserts there is nothing to assume, and that is false.I guarded it with the file's own
IsPositiveRealidiom and ran the suite.work/TRIAGE.mdrecords the cost as "the log-equation solver loses coverage". That is not what it is.SimplifyTest.PowerRulesTestforln(a) + ln(b)andln(a) - ln(b)— the rules' own tests — andOneSidedLimitTest.ADifferenceOfReciprocalLogarithms(Left).--blame-hangthe host is killed with the offending test amongRemarkableLimitAfterSimplificationTest,RealCodomainLimitTest.AgreeingOneSidedLimitsArePromoted,StirlingFactorialLimitTest,PowerQuotientGatheringTestandSolveOneEquation.LinearTrigRoots. Four of those five are limit tests.The dependency is termination, not coverage: the limit machinery expands logarithms (
LogarithmExpanded) and relies on the simplifier to gather them back.Why this is documented rather than fixed here
The precedent is eight rules up in the same file.
a^n / b^n -> (a/b)^nwas guarded for exactly this reason, cost(x^2 + 1)^x / (x^2)^xits limit, and was repaired by teaching the limit reader to recognise the quotient itself — whereIsEventuallyPositivecan require the bases to be positive on the approach. SeeApplySecondRemarkable.That repair also needed a thread-static re-read bound (
MaxSecondRemarkableRereads), because the rewrite feeds itself indirectly and every level asks for limits of its own. The logarithm path has no such bound, which is the likeliest reason its symptom is a hang rather than a lost answer. Landing the same move for logarithms is a real change to the limit machinery and wants its own PR with its own measurements, not a guard bolted onto this one.§11 states the acceptance for it: guard in place,
boundcheckat 0 disagreements, those three tests passing, the suite finishing in its usual five minutes, and no limit answer lost.§10 gains the caution that follows. This rule was the standing example of what per-symbol assumptions (#721's second half) would rescue — and no assumption on
aandbdischarges a termination dependency, because the limit machinery's own expansion is what creates the operands. Assumptions are still worth having; this consumer is not the argument for them.No code changed, so the suite is master's, which is green.