Conversation
tanh was (exp(2x) - 1) / (exp(2x) + 1), whose quotient-rule derivative is two terms near +2 and -2 that cancel; once (exp(2x) + 1)^2 overflows one term drops out and the sum is finite and wrong (t2s: 41.2 where the truth is ~1e-171), or NaN on the scalar side. The value itself was NaN past x~355. Both domains now build tanh as 2 / (1 + exp(-2x)) - 1: still a single exp term, so differentiation never inserts one exp child twice into an add, and the derivative is one product, 4 exp(-2x) / (1 + exp(-2x))^2, which is exact for large positive x and underflows to zero on the negative side. The value is correct for every argument. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
The exp(-2x) form is exact for every positive argument, but below -179 the squared denominator leaves the double range: the derivative over-estimates by up to 1.75x through a denormal intermediate, then underflows to zero, then goes NaN past -354. The earlier tests sampled -50 and -200 and so straddled the band entirely. Both domains now sweep it and pin the degradation one-sided and bounded by 2x, so it cannot widen unnoticed, plus the fold that keeps a syntactically negated argument on the exact branch. Signed-off-by: petlenz <peterlenz89.pl@gmail.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 #480.
Fix level: the composition (option a), with a documented residual.
tanhwas(exp(2x) − 1) / (exp(2x) + 1), whose quotient-rule derivative is two terms near +2 and −2 that cancel; once(exp(2x)+1)²overflows one term drops out and the sum is a finite wrong number on the t2s side, NaN on the scalar side, and the value itself was NaN past x ≈ 355. Both domains now build2 / (1 + exp(−2x)) − 1: still oneexpterm (so the #180 duplicate-insert hazard stays closed), and the derivative is a single product4·exp(−2x)·(1+exp(−2x))⁻²that cannot cancel.Residual — this PR moves a narrow defect from the positive side to the negative side. Below x ≈ −179 the squared denominator leaves the double range and
powreturns a denormal, so the derivative over-estimates: +0.26 % at −185, +29 % at −186, +75 % at −186.25; it is zero from −186.5 (truth ~4e-162) and NaN past −354.25. Main is exact in that band. The trade is deliberate: main's worst undetectable error on the positive side is ~171 orders of magnitude (41.2 vs 1.6e-171), this PR's is a factor of 1.75 in a ~7-wide band.tanh(−y)folds to−tanh(y)and stays exact, so only a bare argument that evaluates negative at runtime reaches it.The symmetric form the review suggested was tried and is worse.
(eˣ − e⁻ˣ)/(eˣ + e⁻ˣ)differentiates to1 − sinh²/cosh², i.e. catastrophic cancellation: at ±186.25 it returns 1.11e-16 where the truth is 6.72e-162 — wrong by 145 orders of magnitude, on both sides, versus 1.75× here. The mirror form1 − 2/(exp(2x)+1)is exactly this PR's behaviour reflected, so it only swaps which sign is wrong. A single-term derivative requires a constant numerator, and no such composition is stable on both sides —4/(eˣ+e⁻ˣ)²is the stable expression but no composition differentiates to it without an algebraic rewrite. Dedicated hyperbolic nodes with asech²rule (#126) are the only complete fix; anif_then_elsesplit was considered and rejected (it would put a branch in every emittedtanh).Tests.
CoreBugFix.TanhStaysSoundForLargeArgumentsandTensorToScalarDifferentiationTest.TanhOfLargeTraceGradientIsSoundpin the fixed behaviour.CoreBugFix.TanhDerivativeDegradesOnlyInTheKnownBandandTanhGradientDegradesOnlyInTheKnownBandsweep −179.25 → −354 in 0.25 steps (0.5 for t2s, whose evaluation is heavier) and assert the degradation is one-sided and bounded by 2×, so it cannot widen unnoticed; the first version of these tests sampled −50 and −200 and straddled the band, which an adversarial review caught.Negative control (both headers restored from main, rebuilt): all four tanh tests fail — the positive-side ones on the value and derivative, the band ones on the far tail and on the negation-fold case (main returns
infthere). The negative-band assertions themselves pass on main, since main is exact there; they are characterisation of this PR's residual, not a demonstration of a main bug, and are labelled as such.Siblings checked at large and extreme arguments in both signs:
sinh,coshgrow consistently to inf;acosh,atanh,log10exact.asinhhas the same class on its negative side (−1e7 → value 0.03 % and derivative 1.5 % wrong; −1e8 → −inf/NaN); a symmetric identity was probed and rejected because it degrades the currently-exact positive side. Filed as #502.Full suite 2409/2409 in Debug and in Release (gcc-14); clang-format clean.
Follow-up: PR #481's fuzz generators bound hyperbolic arguments to dodge this limitation; the
tanhbound can be relaxed to the positive side once this lands, whileasinhkeeps its bound until #502.