fix(stoch,kama): divide by the value the guard tests, and bound the efficiency ratio (#390) - #394
Merged
Conversation
…fficiency ratio (#390) STOCH and STOCHF returned inf/NaN under TA_SUCCESS for well-formed OHLC, and KAMA could leave the range of the prices it was smoothing. One defect shape twice: a guard that does not establish what the arithmetic below it needs. %K guarded `highest-lowest` and divided by `diff = (highest-lowest)/100.0`. A denormal range underflows diff to exactly 0.0 while the band still answers "not flat", so the division runs on zero. That hoist is a 2002 speed optimization and no other library has it -- Tulip, ta-rs, cinar, finta, technicalindicators and yata all divide by the range and scale after. Doing the same removes the mismatch structurally instead of guarding it, deletes `diff` from the body and from the streaming handle, and makes the endpoints exact: a close on the window high is x/x, so %K is exactly 100.0. Over a magnitude sweep including denormals the new form gives 0 non-finite outputs for well-formed bars where the old gives 24, and 270,563 close-at-high cases all return exactly 100.0. The efficiency ratio |periodROC|/sumROC1 cannot exceed 1 -- periodROC telescopes over exactly the changes sumROC1 sums. But sumROC1 is kept by add-then-subtract, so absorption leaves it below the window it stands for and the raw ratio runs unbounded: measured 1.1e18, driving the squared smoothing constant to 4.4e35 and KAMA's output 43 decades outside the hull of its inputs. Clamping restores the bound, so the constant stays in (0,1) and the recurrence is a convex combination again. This overturns er.c's standing "do NOT fix this with fabs": that ruling was about value stability and was made without the unbounded case in evidence. v0.6.4 has neither defect. #253 opened both while making the bands scale-relative -- which it was right about; only the quantity the STOCH band tests moved off the divisor, unremarked. STOCH and STOCHF need no new tolerance: their existing FUZZ_064_TOL rows absorb the change with four orders to spare (max observed 2.27e-12 against 1e-9). KAMA needs one skip, gated on the absorption condition and computed from the inputs alone -- kama-skipped 1089 -> 1139 of 166,852 comparisons -- and every case it drops had v0.6.4 emitting a ratio outside [0,1], on the EXTREME shape, reaching a negative output from an all-positive input. LEGACY_TOL re-measured: STOCH 8.53e-14, STOCHF 4.26e-14, both moved to cause (b). Two legs pin the clamp, each sabotage-proven by deleting it and watching them redden: a KAMA sign-mirror (KAMA is exactly sign-symmetric but for the signed comparison the clamp neutralizes) and an exact-1.0 predicate on ER's zero-denominator leg. The divisor sweep's self-test now reconstructs the defect rather than pointing at STOCH, and pins all three of its mechanisms. Green: full C suite, the generator's cargo test, --xlang-hash (52,000 cases, four backends bit-identical), --codegen against the frozen oracle (6 passed 0 failed in each backend), and fuzz-064. Shipped-build A/B on a Zen 4 laptop, min of 12 alternating rounds, five untouched controls inside +-0.6%: STOCH 0.990, STOCHF 0.958, KAMA 1.050. The per-bar clamp costs KAMA about 5%. CCI, CORREL and WILLR carry the same guard/divisor mismatch and are NOT fixed here. Claude-Session: https://claude.ai/code/session_01Mxrh2CPbhZnLvtzepLeCwt
…ource fixture `retarget` never changes the length, so `&mut Vec<Statement>` trips `clippy::ptr_arg` under the generator's `-D warnings`. Claude-Session: https://claude.ai/code/session_01Mxrh2CPbhZnLvtzepLeCwt
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 #390.
Two functions, one defect shape: a guard that does not establish what the arithmetic below it needs.
STOCH / STOCHF
%Kguardedhighest-lowestand divided bydiff = (highest-lowest)/100.0. A denormal range underflowsdiffto exactly0.0while the band still answers "not flat", so the division runs on zero —inf/NaNreturned underTA_SUCCESS, which then poisons every later bar through the%K/%Dsmoothing.Rather than add a second guard, this divides by the range itself and scales after. That hoist is a 2002 speed optimization and no other library has it — Tulip, ta-rs, cinar/indicator, finta, technicalindicators and yata all compute
(c-l)/(h-l)*100. Matching them removes the mismatch structurally, deletesdifffrom the body and from the streaming handle, and makes the endpoints exact: a close on the window high isx/x, so%Kis exactly100.0.Measured over a magnitude sweep including denormals: 0 non-finite outputs for well-formed bars where the old form gives 24, and 270,563 close-at-high cases all returning exactly
100.0.KAMA / ER
The efficiency ratio
|periodROC|/sumROC1cannot exceed 1 —periodROCtelescopes over exactly the changessumROC1sums. ButsumROC1is kept by add-then-subtract, so absorption leaves it below the window it stands for and the raw ratio runs unbounded: measured 1.1e18, driving the squared smoothing constant to 4.4e35 and KAMA's output 43 decades outside the hull of its own inputs. Clamping restores the bound, so the constant stays in(0,1)and the recurrence is a convex combination again.This overturns
er.c's standing "do NOT fix this with fabs". That ruling was about value stability and was made without the unbounded case in evidence.Cost against v0.6.4
v0.6.4 has neither defect; #253 opened both while making the bands scale-relative — which it was right about, only the quantity the STOCH band tests moved off the divisor, unremarked.
FUZZ_064_TOLrows absorb the change with four orders to spare (max observed 2.27e-12 against 1e-9).kama-skipped1089 → 1139 of 166,852 comparisons. Every case it drops had v0.6.4 emitting a ratio outside[0,1]on the EXTREME shape — in the worst one, a negative output from an all-positive input, from a function flaggedTA_FUNC_FLG_OVERLAP.Verification
Full C suite · generator
cargo test·--xlang-hash(52,000 cases, four backends bit-identical) ·--codegenagainst the frozen oracle (6 passed / 0 failed in each backend) ·fuzz-064·regen-checkon a clean tree.LEGACY_TOLre-measured — STOCH 8.53e-14, STOCHF 4.26e-14 — and both rows moved to cause (b), whose prose the change made false where they were.Two legs pin the clamp, each sabotage-proven by deleting it and watching them redden: a KAMA sign-mirror (KAMA is exactly sign-symmetric but for the signed comparison the clamp neutralizes) and an exact-
1.0predicate on ER's zero-denominator leg. The divisor sweep's self-test (#392) now reconstructs the defect rather than pointing at STOCH, and pins all three of its mechanisms.Performance
Shipped-build A/B (
libta-lib.a, separate TUs, no LTO), Zen 4 laptop, one function per process, min of 12 alternating rounds:Controls inside ±0.6%. STOCH and STOCHF get slightly faster; the per-bar clamp costs KAMA about 5%.
ta_bench --language=cwas tried first and rejected: it reported STOCH +7.4% and STOCHF −18.0% from the identical edit, with spreads of 30–51% — the single-TU LTO inlining instability theta-benchskill documents.Not fixed here
CCI, CORREL and WILLR carry the same guard/divisor mismatch and are out of scope. Reproduced through the shipped API: CCI returns 39 of 67 outputs
infunderTA_SUCCESSat denormal quote scale; CORREL returnsNaN; WILLR divides residue into the full range on the machine-flat window where STOCHF now returns zeros, despite being documented as bounded in[-100, 0]. Follow-up issue to come.https://claude.ai/code/session_01Mxrh2CPbhZnLvtzepLeCwt