fix: quantile-panelled quadrature for Compete's wide-window accuracy (#294)#298
Open
seabbs-bot wants to merge 1 commit into
Open
fix: quantile-panelled quadrature for Compete's wide-window accuracy (#294)#298seabbs-bot wants to merge 1 commit into
seabbs-bot wants to merge 1 commit into
Conversation
…294) probs/mean/var on a racing-hazard (Compete) node integrated over the full shared quadrature window with a single fixed 64-node Gauss-Legendre rule. Once that window is wide (a heavy-tailed cause with a finite but huge quantile, e.g. Pareto(0.5, 1.0); or an ordinary composite cause whose moment-based fallback window is itself large), the fixed nodes spread across the whole domain and starve the region where the mass actually sits, returning a badly wrong split (off by 100%, potentially naming the wrong winning cause) while still looking plausible. The quadrature now splits the window at each cause's own quantile (or, lacking a quantile method, mean/std) markers and integrates panel-wise, so node density follows the mass regardless of which cause's tail set the window -- mirroring the quantile-panelled approach ConvolvedDistributions already uses internally for its own analogous convolution quadrature. _HazardCauseDelay's cdf (the forward convolve stream) benefits the same way, since its integration bound can be arbitrarily far out for a heavy-tailed node. Verified against QuadGK's adaptive quadrature during implementation: the new split matches to ~10 significant figures for the Pareto case and closely for the skewed-composite case, confirming the previous PR's own "converged" reference (a single 65536-node rule over the raw window) was itself not reliably converged for the most extreme case -- the affine map onto such a wide interval is ill-conditioned regardless of node count. Flips the four @test_broken assertions from #288's delta round to real, passing assertions, replacing the Pareto case's reference with one integrated over a smaller, well-conditioned truncated domain (justified: the race resolves once the faster cause's own survival is negligible, well before the heavy-tailed cause's extreme quantiles). Closes #294.
Contributor
Try this Pull Request!Option 1: Julia Package ManagerOpen Julia and type: import Pkg
Pkg.activate(temp=true)
Pkg.add(url="https://github.com/EpiAware/ComposedDistributions.jl", rev="fix/cd-294-quad-accuracy")
using ComposedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout fix/cd-294-quad-accuracy
julia --project=. -e "using Pkg; Pkg.instantiate()" |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Contributor
|
📖 Documentation preview is ready! View the docs for this PR at: https://EpiAware.github.io/ComposedDistributions.jl/previews/PR298/ This preview will be updated automatically when you push new commits. |
Contributor
Benchmark comparison vs baseMinimum time per call. Buckets are PR time as a % of base, so lower is faster (🟢 faster, ⚪ within 5%, 🔴 slower). Counts of benchmarks per bucket:
Evaluation — 17 benchmarks (by time change)
AD gradients — 28 benchmarks (by time change)
|
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
1 participant
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.
Summary
probs/mean/varon a racing-hazard (Compete) node integrated overthe full shared quadrature window with a single fixed 64-node
Gauss-Legendre rule. Once that window is wide (a genuinely heavy-tailed
cause with a finite but huge quantile, e.g.
Pareto(0.5, 1.0); or anordinary composite cause whose moment-based fallback window is itself
large), the fixed nodes spread across the whole domain and starve the
region where the mass actually sits, returning a badly wrong split (off
by 100%, potentially naming the wrong winning cause) while still looking
plausible (finite, in
[0, 1], summing to<= 1).lacking a
quantilemethod, mean/std) markers and integrates panel-wise,so node density follows the mass regardless of which cause's tail set
the window (
_hazard_panelled_integrate) — mirroring thequantile-panelled approach ConvolvedDistributions already uses
internally for its own analogous convolution quadrature (those helpers
are that package's internals, not public, so this reimplements the same
idea directly against the public
GaussLegendre/integrateAPI, andpools breaks from every cause since a race's mass can concentrate around
any cause's own scale, not just the one whose quantile set the window).
_HazardCauseDelay'scdf(the forwardconvolve_seriesstream)benefits the same way, since its integration bound can be arbitrarily
far out for a heavy-tailed node.
probscomputes the panel breaks once per node and reuses them acrossevery cause's integral, rather than recomputing the same ladder per
cause.
Closes #294.
Verification note
Independently checked against
QuadGK's adaptive quadrature duringimplementation (not a project dependency, used only to validate): the new
split matches to ~10 significant figures for the Pareto case and closely
for the skewed-composite case. This also showed that #288's own
"converged" reference for the Pareto case (a single 65536-node rule over
the raw ~1e8 window) was itself not reliably converged — the affine map
onto such a wide interval is ill-conditioned regardless of node count
(checked up to
n = 1_048_576, still drifting). The test's reference forthat case now integrates over a smaller, well-conditioned truncated
domain instead, justified because the race's winning-probability integral
is bounded by whichever cause's own survival is smallest: once the faster
cause has essentially resolved, the joint survival is already negligible,
well before the heavy-tailed cause's own extreme quantiles.
Test plan
@test_brokenassertions from fix: racing-node quadrature window robust to composite/heavy-tailed causes (#259) #288's delta round toreal, passing assertions (Pareto split accuracy, skewed-composite
split accuracy, and the winner-flip).
task test-fast— 1590/1590 passed.task test-quality— 234/234 passed.task test-formatting— all files properly formatted.This was opened by a bot. Please ping @seabbs for any questions.