Repository navigation
feat: add total_mass/is_defective accessors for sub-stochastic laws - #108
Conversation
A negative additive-hazard effect (or a clamped numeric hazard modification) can leave a Modified law sub-stochastic, its cdf converging to a value below one. total_mass/is_defective expose that deficit as a public, queryable surface, so a caller can inspect it before a quantile/rand query that would otherwise throw. Closes #107. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
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/ModifiedDistributions.jl", rev="feat/total-mass-defective-107")
using ModifiedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout feat/total-mass-defective-107
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.
🚀 New features to boost your workflow:
|
|
📖 Documentation preview is ready! View the docs for this PR at: https://EpiAware.github.io/ModifiedDistributions.jl/previews/PR108/ This preview will be updated automatically when you push new commits. |
ReviewVerified the three exact-
Finding (structural, numeric-path): I constructed an adversarial base with a genuine bathtub hazard (a hazard that dips below a clamp level and rises back above it beyond the deep-quantile window): mix = MixtureModel([Exponential(0.05), Weibull(3.0, 50.0)], [0.9999, 0.0001])
d = modify(mix, -0.3; link = identity)
# scan_upper(mix) ≈ 104.8; hazard crosses back above the 0.3 clamp level
# somewhere in (110, 130) — beyond that window.Comparing
In this instance ccdf/cdf consistency: confirmed exact for a genuinely defective case ( AD/hot-path isolation: Minor doc gap: Suites: reproduced Overall: the core design and the three exact-mass proofs are sound. The numeric-path finding is a real gap worth a decision (doc caveat vs. widened scan vs. accepted limitation) rather than a blocker, and the tolerance-documentation fix is small. |
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 — 40 benchmarks (by time change)
AD gradients — 66 benchmarks (by time change)
|
|
Triage: blocked on a maintainer ruling — please do not merge yet. Nothing here needs changing; the open question is where the verb lives. The pending rulingEpiAware/ComposedDistributions.jl#252 asks for "one defective type that composes everywhere", and states that "the single sub-stochastic type should be shared across all producers rather than reinvented at each site". Two things are still undecided:
Why hosting here is a real problem, from the dependency graphRead off the ComposedDistributions does not depend on ModifiedDistributions, in either direction — the only edge between them is ModifiedDistributions' weak dependency, which powers this repo's own extension. So a generic defined and exported here is unreachable from ComposedDistributions: CD cannot write Three consequences of merging as-is:
What was verifiedThe implementation itself checks out on the points not touched by the ruling. Two small items already raised in the review thread still stand against the current diff:
Both are one-line documentation fixes and are not the blocker. Related#110 stacks on this branch and extends this generic with This was opened by a bot. Please ping @seabbs for any questions. |
|
It looks like this needs to be implemented in composeddistributions and then extended here? Or both packages need to depend on a simple package that has simple distribution based concepts in it? |
…tive-107 # Conflicts: # NEWS.md # src/ModifiedDistributions.jl
_defective_total_mass looped, doubling the query point past a deep base quantile until successive cdfs agreed to _DEFECTIVE_TOL. In practice a single doubling is always enough: the deficit at quantile(dist, 1 - 1e-12) is already below Float64's resolution near one, so a further doubling can never move the returned cdf by more than the tolerance. The loop's continuation/exhaustion branches were consequently unreachable dead code, which is what left codecov/patch failing on #108 regardless of what was tested. Dropped the loop; behaviour is unchanged (full suite still passes, including the existing defective and truncated-support cases). Also added a test exercising total_mass/is_defective through the numeric cumulative-hazard path (_NumericModified, a callable effect), which the original PR only exercised implicitly - the closed-form identity-link and discrete-per-bin paths were tested directly, the numeric path was not.
piecewise_effect(; breaks, multipliers) is a piecewise-constant proportional-hazards multiplier for modify under the log link, with a closed-form cumulative hazard (a sum of base log-survival differences over segments) rather than the numeric quadrature path a callable effect(t) always takes. Segment i applies from breaks[i-1] up to (but not including) breaks[i], matching the right-continuous step-function convention the issue's own illustrative example uses. gate(dist, opens, closes) is a finite-window special case (multipliers 0/1/0 at the opens/closes breakpoints): the hazard is off before opens, runs unmodified inside the window, and switches off again at closes, leaving a deliberately sub-stochastic law whose residual "never occurred" mass reads through total_mass/is_defective (#107) like any other defective wrapper. Stacked on the not-yet-merged #107 branch (PR #108) since gate reuses its total_mass/is_defective accessor directly; total_mass gets one additional dispatch for the piecewise path (exact when the trailing multiplier is zero, the general numeric search otherwise). Closes #105. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
Summary
total_mass(d)/is_defective(d)for amodifyd law: a public,queryable surface for the sub-stochastic (defective) laws a negative
additive-hazard effect (or a clamped numeric hazard modification) can
produce.
total_massis exact (1) for the proportional-hazards, non-negativeadditive-hazard and discrete per-bin paths (which never clamp away mass),
and numerically searched (evaluated exactly at the base's own finite
maximum when bounded, otherwise a doubling cap search mirroring the
bracket the
quantilemethods already expand internally) for the clampedidentity-link and numeric cumulative-hazard paths.
ccdfalready reports the same residual deficit consistently(
ccdf(d, x) -> 1 - total_mass(d)asxgrows); tested directly.Scope note
Deliberately scoped to
Modifiedonly — not peeled throughAffine/Weighted/Transformed. This is a foundation stone for the cross-orgdefective-law epic (ComposedDistributions#252); the general
multi-wrapper accessor surface (e.g.
total_massof aweight-wrappedModified) is left to that epic's design rather than guessed here.Closes #107.
Test plan
task test-fast— 2170/2170 pass (new cases: proper laws neverdefective across log/identity/discrete paths; negative-additive clamp
matches the existing plateaued-cdf fixture; finite-support base evaluated
exactly; ccdf/cdf consistency).
task test-quality— Aqua/formatting/doctests pass (109/109).task test-ad— all AD backends pass (1386/1386).This was opened by a bot. Please ping @seabbs for any questions.