Skip to content

Fix StackOverflow in condition() for non-Float64 values#368

Merged
lrnv merged 1 commit into
lrnv:mainfrom
thisiscam:fix-condition-bigfloat
May 31, 2026
Merged

Fix StackOverflow in condition() for non-Float64 values#368
lrnv merged 1 commit into
lrnv:mainfrom
thisiscam:fix-condition-bigfloat

Conversation

@thisiscam
Copy link
Copy Markdown
Contributor

condition(C, js, uⱼₛ) and its SklarDist method type the conditioning values as NTuple{p, Float64}. The untyped entry point routes through _process_tuples, which normalises with float. — but float(::BigFloat) === BigFloat and float(::Float32) === Float32, so non-Float64 reals miss the typed method, fall back to the untyped entry point, and recurse forever:

julia> condition(ClaytonCopula(3, 2.0), (1, 2), (big"0.3", big"0.4"))
ERROR: StackOverflowError:

(also reproduces with Float32, and via the scalar / SklarDist entry points).

Fix: widen both typed methods to NTuple{p, <:Real}. Float64 is still <:Real, so the existing Float64 path is unchanged — the method is replaced, not added alongside, so there is no new ambiguity. Non-Float64 values are converted to Float64 by the downstream DistortionFromCop/ConditionalCopula fields, so conditioning is still computed in Float64 regardless of input precision (making the conditioning machinery itself precision-generic would be a larger, separate change).

Adds a regression test covering the Copula and SklarDist entry points, scalar and tuple js, single- and multi-conditioned dims, with BigFloat and Float32 values.

Found while building nested-Archimedean support (#367), where conditioning on BigFloat coordinates is natural.

`condition(C, js, uⱼₛ)` (and the SklarDist method) typed `uⱼₛ` as
`NTuple{p,Float64}`. The untyped entry point routes through `_process_tuples`,
which calls `float.` — and `float(::BigFloat) === BigFloat`,
`float(::Float32) === Float32` — so non-`Float64` reals miss the typed method,
fall back to the untyped entry point, and recurse forever (StackOverflow).
Reproduces on a plain `condition(ClaytonCopula(3, 2.0), (1, 2), (big"0.3", big"0.4"))`.

Widen both typed methods to `NTuple{p,<:Real}`. `Float64` is still `<:Real`, so
existing dispatch and behaviour are unchanged (the method is replaced, not
added — no new ambiguity). Non-`Float64` values are converted to `Float64` by
the downstream `DistortionFromCop`/`ConditionalCopula` fields, so the result is
computed in `Float64` regardless of input precision.

Adds a regression test covering Copula and SklarDist entry points, scalar and
tuple, single- and multi-conditioned dims, with BigFloat and Float32 values.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lrnv
Copy link
Copy Markdown
Owner

lrnv commented May 30, 2026

Thanks a lot for this. Also related is #195 : the promise of type-agnosticity on the front page of the package is not really repected in most of our code. Fixing it will require many changes and i keep pushing back ;) It's nice that NACs already support it :)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.28%. Comparing base (89c9c18) to head (50a8026).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #368      +/-   ##
==========================================
- Coverage   78.30%   78.28%   -0.02%     
==========================================
  Files          84       84              
  Lines        5106     5106              
==========================================
- Hits         3998     3997       -1     
- Misses       1108     1109       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lrnv lrnv merged commit 5cd3d24 into lrnv:main May 31, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants