chore(deps): upgrade to ConvolvedDistributions 0.4 - #147
Draft
seabbs-bot wants to merge 3 commits into
Draft
Conversation
Bumps the ConvolvedDistributions compat bound to "0.2, 0.3, 0.4" across the root, test, docs, benchmark, test/ad and test/ADFixtures environments (supersedes dependabot #139's narrower ConvolvedDistributions edit). The actual resolved version stays at 0.3.1 for now: ComposedDistributions (a co-dependency in the shared test/docs/benchmark environments, used for the separate Composed x Convolved integration tests) has not yet widened its own ConvolvedDistributions compat past 0.3 in either its registered release (0.1.1) or its unreleased main branch (0.2.0-dev, compat "0.2, 0.3, 1" — the gap between 0.3 and 1.0 excludes 0.4.x). This bump is a safe no-op until that upstream gap closes; it does not change what CI actually resolves or runs today.
ConvolvedDistributions 0.4 removed the checked ArgumentError gate on convolve_series(continuous_delay, series) (CD#95): an unwrapped continuous delay now simply matches no method, so the call raises a plain MethodError instead. Verified directly against a local ConvolvedDistributions 0.4.1 checkout (registry has 0.4.0): the same call that threw ArgumentError under 0.2/0.3 now throws MethodError. This env cannot yet resolve ConvolvedDistributions 0.4 end to end (see the previous commit), so the assertion still runs against 0.3.1 today; widening it to accept either exception type keeps the test correct across the whole declared compat range once CD 0.4 becomes reachable.
ConvolvedDistributions 0.4 added convolve_series(delays::AbstractVector,
series) — one delay per time point, dispatched element-by-element
through the newly public delay_masses hook. Ran it directly against a
real ConvolvedDistributions 0.4.1 checkout: delay_masses's default
bounces back through a delay's own single-delay convolve_series, which
already reaches our Transformed/AbstractModifiedDistribution methods,
so a Vector of forward-wrapped delays (thin, in particular) composes
correctly with no extra method needed, including through
ConvolvedDistributions' ===-keyed masses de-duplication cache for
repeated delays. Added a regression test for this, gated on
pkgversion(ConvolvedDistributions) since the vector form and
delay_masses do not exist before 0.4 (confirmed absent from the 0.2/0.3
source) — it runs for real once CD 0.4 is reachable in this env (see
the previous two commits) and skips cleanly today.
Also audited and documented, in the extension itself, the remaining CD
0.4 changes flagged for this upgrade:
- delay_masses vs our private _discretised_masses: not the same thing.
delay_masses's default has no continuous-discretisation logic of its
own (a bare continuous delay still MethodErrors through it) and its
signature has no interval grid-width argument ours exposes. Left
_discretised_masses as is, and did not add a delay_masses method
(the symbol does not exist pre-0.4, so importing it unconditionally
would break loading against this package's still-supported 0.2/0.3
compat range for zero behavioural gain over the default).
- _has_batched_method and the new Ratio type: checked with
which(logpdf/pdf/cdf, Tuple{Ratio, Vector{Float64}}) (also Product,
Difference) — all resolve to Distributions.jl's generic elementwise
fallback, not a ConvolvedDistributions-owned batched method. No
declaration added.
- Solver dispatch for analytic pairs / exact discrete support: confirmed
with ConvolvedDistributions.evaluation_path that our wrapper types
still take the :numeric route (that dispatch keys on concrete
Distributions.jl types), so the primal_distribution quadrature-window
reconstruction stays necessary and correct under 0.4.
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/ModifiedDistributions.jl", rev="feat/convd-04-upgrade")
using ModifiedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout feat/convd-04-upgrade
julia --project=. -e "using Pkg; Pkg.instantiate()" |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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 — 40 benchmarks (by time change)
AD gradients — 66 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
Audits this package's ConvolvedDistributions (CD) integration against the
six changes in CD 0.4.0 (released 2026-08-06) and widens the compat bound.
Supersedes dependabot #139's narrower ConvolvedDistributions edit (that PR
also bundles an unrelated EpiAwarePackageTools bump, left untouched here).
What broke
Nothing in
ext/ModifiedDistributionsConvolvedDistributionsExt.jlitself.Verified against a real ConvolvedDistributions 0.4.1 checkout (registry has
0.4.0) in an isolated scratch environment: all 96 assertions in
test/ConvolvedDistributionsExt.jlpass (94 pre-existing + 2 new, seebelow).
One test assertion elsewhere was pinned to CD's old behaviour and needed
updating:
test/integration.jlassertedArgumentErrorfrom a bareconvolve_series(continuous_delay, series)call — CD 0.4 removed thatchecked gate (CD#95), so the call now raises a plain
MethodErrorinstead. Fixed to accept either, so the assertion holds across the whole
0.2, 0.3, 0.4compat range.What I checked, one item per CD 0.4 change — all verified by running, not just reading
Continuous-delay gate removed (
ArgumentError->MethodError).Confirmed directly:
convolve_series(convolved(...), series)throwsMethodErrorunder real CD 0.4.1,ArgumentErrorunder 0.3.1. Onlytest/integration.jlexercised the bare gate directly; fixed (above).convolve_seriesnow takes a vector of per-time-point delays (newin 0.4 — confirmed absent from the 0.2/0.3.1 source). Ran the exact
scenario against real CD 0.4.1: a
Vectorofthin-wrapped delays withdifferent thin factors, including the same base delay recurring
with different factors (to exercise CD's
===-keyed massesde-duplication cache). Result matched the hand-computed "ops applied"
expectation exactly and diverged from the "ops silently dropped"
hypothesis — confirming CD's new
delay_masseshook's default (whichbounces back through a delay's own single-delay
convolve_series)correctly reaches our
Transformed/AbstractModifiedDistributionmethods via ordinary dispatch, so forward ops are not lost. No code
change needed; added a regression test
(
test/ConvolvedDistributionsExt.jl, gated onpkgversion(ConvolvedDistributions)since the method doesn't existpre-0.4) plus documentation in the extension itself.
Internal renames (
_causal_convolve->_convolve_series_fixed,_convolve_series->_convolve_series_varying). Grepped the wholerepo: neither name is referenced anywhere. Not applicable.
delay_masseshook now public. Compared against our private_discretised_masses: not the same thing.delay_masses's default hasno continuous-discretisation logic of its own (a bare continuous delay
still
MethodErrors through it — same as point 1) and its signature(
delay_masses(d, n::Int)) has nointervalgrid-width argument oursexposes and callers use. Left
_discretised_massesas is. Deliberatelydid not add a
delay_massesmethod for our wrapper types: thesymbol does not exist in CD 0.2/0.3 (verified via CD's git history —
introduced by the same commit series as the 0.4 vector feature), so an
unconditional
import ConvolvedDistributions: delay_masseswould breakextension loading against this package's still-supported 0.2/0.3
compat range — for zero behavioural gain, since point 2 already showed
the default composition gives the identical, correct result.
New
Ratiotype (+pgf,quantile_by_optimization). Checked atruntime, not just by reading source:
which(logpdf/pdf/cdf, Tuple{Ratio, Vector{Float64}})(alsoProduct,Difference) all resolve toDistributions.jl's generic elementwisefallback (
deprecated.jl), not a ConvolvedDistributions-owned batchedmethod — only
Convolvedhas one (Convolved.jl). Left_has_batched_methodcovering onlyConvolved; a false declarationwould have routed a whole vector into a method that doesn't exist.
Solver dispatch for analytic pairs / exact discrete support.
Checked with
ConvolvedDistributions.evaluation_pathon the existing"modifiers as convolution components" scenarios (an affine-wrapped
Gamma component, an affine-wrapped unbounded Normal as the last
component): both still report
:numericunder CD 0.4 — that newdispatch keys on concrete Distributions.jl types (e.g.
Normal+Normal), not our wrapper types, so ourprimal_distributionquadrature-window reconstruction stays necessary and is still exercised
and correct.
Why the resolved version stays at 0.3.1 for now
Bumping the ConvolvedDistributions compat bound here is necessary but not
sufficient:
ComposedDistributionsis a co-dependency in the sharedtest/docs/benchmark environments (for the separate Composed x Convolved
integration tests), and its own ConvolvedDistributions compat — both the
registered 0.1.1 and the unreleased main branch (0.2.0-dev, compat
"0.2, 0.3, 1") — has a gap between 0.3 and 1.0 that excludes CD's0.4.x. So this repo's compat bump is a safe no-op today:
Pkg.resolve()still picks ConvolvedDistributions 0.3.1, and the full test suite
(2194 pass + 1 skip + 116 quality) passes against that. It'll pick up 0.4
automatically once ComposedDistributions widens its own bound — no
further change needed on this side. The new gated test (point 2 above)
will start actually running once that happens.
I looked at re-pinning ComposedDistributions to its main branch via
[sources](the pattern this repo used for the earlier CD 0.2 pin,ModifiedDistributions #87) but reverted it: main's own compat already
excludes CD 0.4.x too, so the pin doesn't unblock anything and only adds
an extra unregistered git dependency.
What I left alone
unrelated to this task, left for that PR (or a separate one) to handle.
delay_massesmethod for our modifier types (point 4 above) —correct by construction to add, but unsafe to add unconditionally given
the current compat range, and there's no behavioural gap to justify the
version-gating complexity.
Test plan
test/ConvolvedDistributionsExt.jl(96 assertions, including thenew time-varying-vector test) passes against a real
ConvolvedDistributions 0.4.1 in an isolated scratch env
skip_quality): 2194 pass, 1 skipped (the newversion-gated test, correctly skipping against the resolved 0.3.1)
quality_only, Aqua/formatter/etc.): 116/116 passPkg.resolve()from a clean Manifest confirms no accidentalpins
This was opened by a bot. Please ping @seabbs for any questions.