feat: open node/leaf composability, add the #277/#278 conformance harness - #365
feat: open node/leaf composability, add the #277/#278 conformance harness#365seabbs-bot wants to merge 5 commits into
Conversation
ConvolvedDistributions 0.4.0, EpiAwareADTools 0.1.2, EpiAwarePackageTools 0.3.0, ModifiedDistributions 0.1.0 and DistributionsInference 0.1.0 are all in General, so no environment needs a git pin to resolve. The root fix alone was not enough. ConvolvedDistributions main moved to 0.4.0 while the workspace members still bounded it at "0.2, 0.3, 1", giving 'empty intersection between ConvolvedDistributions@0.4.0 and project compatibility [0.2 - 0.3, 1]'. Every Test, AD, Documenter and coverage job on main and on all open PRs failed at instantiate. - test/, docs/, benchmark/ and test/ad/ now bound ConvolvedDistributions at "0.4, 1", matching the root. - Drop the git pins for ConvolvedDistributions, EpiAwareADTools and EpiAwarePackageTools from test/, docs/, benchmark/ and test/ad/. Only the path sources for the package itself and ADFixtures remain. - Add the compat bounds those pinned deps were missing, so a registered release is named rather than implied. Gates: all four environments instantiate clean, test-fast 1697/1697, test-quality 239/239.
…ness Widen the closed-type composability gates (`_is_composable`, `_is_one_of_branch`, `_compose_child`, `_is_composer_node`) to dispatch structurally on the public `AbstractComposedDistribution` root instead of a `Union` of the built-in types, so a downstream node subtyping it and implementing `child_nleaves`/`child_logpdf`/`child_rand!` nests as a named child of every built-in with no registration. `has_varying`/`has_uncertain` widen the same way, riding a newly-public `node_children` accessor (renamed from the internal `_node_children`, alias kept). Fix `leaf_mean`/`leaf_var`: they used to peel through `free_leaf` for every leaf, silently reporting a `Truncated`/`Censored` leaf's untruncated moment. `Truncated`/`Distributions.Censored` now try their own `mean`/`var` first and fall back to the old approximation only when Distributions.jl has no closed form for that inner family. Add `TestUtils.test_leaf_protocol_completeness` and `TestUtils.test_sampling_consistency`, closing #277 and #278: both name the specific missing hook/assert failure on a deliberately incomplete/corrupted fixture (tested). Merge `interface-contracts.md` + `leaf-protocol.md` into one `docs/src/developer/extending.md`, fix the `@ref new-composer-node` anchor `nesting.jl` already links (pointed nowhere), and make the worked node example actually compose as a child of a built-in. This was opened by a bot. Please ping @seabbs for any questions.
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="feat/node-surface-conformance-harness")
using ComposedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout feat/node-surface-conformance-harness
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/ComposedDistributions.jl/previews/PR365/ This preview will be updated automatically when you push new commits. |
The Truncated{Gamma} example ran without erroring but silently exercised
the pre-fix fallback path (Gamma has no Distributions.jl closed-form
truncated moment), so it never actually demonstrated the leaf_mean fix
the surrounding prose claims. Switch to truncated(Normal; lower), which
does have a closed form and visibly differs from the untruncated mean
(0.7979 vs 0.0), matching the regression test in test/interfaces.jl.
|
Reviewed. Two notes from outside the workflow. The One thing that branch has which this does not. A leaf with vector-valued parameters — On the scope notes. The honesty is appreciated and the calls look right. The two follow-ups you flagged are now tracked: #368 for the unflatten dual's matching inference gap, and #369 for the four underscore-prefixed public Pool helpers, including the DistributionsInference coordination that stopped you renaming them here. On "could not identify or reach sibling agents". Correct that there was nothing to find — the other two chains had not opened PRs when you started. Building off #359 was the right call given main could not instantiate. This comment was posted by a bot. Please ping @seabbs for any questions. |
|
Superseded by #381, which folds this and seven other PRs into one branch off This branch is preserved and the PR body is kept for its rationale — the detail here is the record of why, which #381 summarises rather than repeats. Closing so it does not compete for review. This comment was posted by a bot. Please ping @seabbs for any questions. |
2 participants
What changed
Structural composability.
_is_composable,_is_one_of_branch(nesting.jl),_compose_child/compose(origin; ...)(compose.jl), and_is_composer_node(Shared.jl) now dispatch on the publicAbstractComposedDistributionroot instead of a closedUnion{Sequential,Parallel,Choose,...}. A downstream node subtyping that abstract and implementingchild_nleaves/child_logpdf/child_rand!nests as a named child of every built-in (sequential/parallel/choose/resolve/compete/compose) with no registration, and its flat-vectorlogpdf/child_rand!work through the tree unchanged.has_varying/has_uncertainwidened the same way, riding the existing internal children-accessor, now renamed to publicnode_children(the old_node_childrenkept as aconstalias for internal call sites, matching the package's established rename pattern). A downstream node gets both predicates for free once it definesnode_children.Bug fix:
leaf_mean/leaf_var. These used to peel throughfree_leaffor every leaf, silently reporting aTruncated/Censoredleaf's untruncated moment (measured:leaf_mean(truncated(Normal(0,1); lower=0.0))gave0.0against a true value of0.7979).Truncated/Distributions.Censorednow try their ownmean/varfirst — correct when Distributions.jl provides a closed form — and fall back to the old approximation only when it does not (measured:Truncated{Normal}/Truncated{Exponential}have one,Truncated{Gamma}does not and would otherwise throw a confusingiterateMethodError).New
TestUtilsharness closing Leaf-protocol completeness conformance check #277 and Add a sampling-versus-density consistency check to the public test utilities #278.test_leaf_protocol_completeness(wrap::Function; leaf, prior, param, tag)builds anuncertain-attached and ashared-tagged leaf, applies the caller's wrapper constructor, and asserts the prior/tag/moments survive — naming the specific missing hook on failure.test_sampling_consistency(d; nsamples, rng, cdf_tol_factor, moment_se)draws a large sample and checks the empirical CDF/mean/var against the analytic ones at a tolerance scaled to sample size. Both are proven to actually catch the bug they're meant to catch (a deliberately incomplete wrapper, a deliberately corruptedrand), not just pass on everything.Docs. Merged
interface-contracts.md+leaf-protocol.mdinto onedocs/src/developer/extending.md. Fixed the@ref new-composer-nodeanchor thatnesting.jl's own docstrings already link to (it pointed nowhere — the page didn't exist). Made the worked node example (Both) actually compose as a named child of a built-in, which it did not before (it was a bare struct, not anAbstractComposedDistributionsubtype, so the composability gate above rejected it). The "what this does not yet cover" section says plainly that the flat-vector named output (rand(tree)as a NamedTuple) and codec (flat_dimension/flatten/unflattenforuncertainparameters on a brand-new multi-leaf node type) still only work for the built-ins — measured, not assumed — and points at Flat codec can't be extended for a leaf whose params aren't its own fields (_params_arity_of / _param_names_of) #332 as the tracked follow-up.What this deliberately does NOT do
children/rebuild/free_parameters) explored in earlier design work — too large and risky for one PR on its own, depends on feat: inner_dist peel hook — collapse leaf-wrapper forwarding boilerplate #362/feat: split rebuild_leaf from leaf_signature #346/feat: full node/attribute/param table with Tables.jl-forwarding tree #343 landing first, and needs dedicated hot-path benchmarking that a single PR pass can't responsibly absorb.MethodErrorchain distinct from the composability gate this PR fixes), documented honestly rather than silently left broken or silently claimed fixed._centred_pool_rowsetc.) — checked,DistributionsInference.jl's extension still calls them directly by that name, so removing now would break DI with no coordinated PR. Also out of scope on its own terms: this is an inference-engine-facing surface, not the node/leaf extension contract "adding a node or a leaf" is about.Gates
task test-fast: 1748/1748 passed.task test-quality: 241/241 passed (Aqua, ExplicitImports, docstring format, doctest, formatting, JET, extension ambiguities).task docs(full build, notdocs-fast): clean — 86 public + 21 internal API bindings generated, no broken cross-references, no@examplefailures.Breaking surface
Nothing removed. Additive/value-correctness only:
node_childrenis a new public name.leaf_mean/leaf_varchange value forTruncated{Normal},Truncated{Exponential}, andDistributions.Censoredleaves (wrong → correct). Anything pinning the old (wrong) number needs updating — flagged here since 0.2.0 is unregistered and no deprecation cycle is owed, but this is a real numeric change worth a release-notes line.docs/src/developer/interface-contracts.mdanddocs/src/developer/leaf-protocol.mdare deleted, merged intoextending.md. Any external link to those pages/anchors breaks.Note on "PR A"/"PR B"
I could not identify or reach sibling agents working the other two parts of this three-part workflow, and no additional draft PR beyond the existing #359/#362/#346/#343 chain existed in this repo when I started. This branch is built off #359 (
chore/drop-convolved-sources-pin) purely to get a working local environment — main does not currently instantiate without it (ConvolvedDistributions@0.4.0vs. the un-widened[compat]bound) — and against current main's actual hook names (free_leaf,uncertain_specs,child_nleaves, etc.), not the hypotheticalchildren/rebuild/inner_distnames an earlier design exploration proposed, since none of that has landed. I did not touchintrospection.jl's table-walk internals that #343 rewrites, only itsnode_children/leaf-moment/public-docstring regions, so this should rebase cleanly once #359 (and, if they are "PR A"/"PR B", #362/#346/#343) land.This was opened by a bot. Please ping @seabbs for any questions.
Closes #277.
Closes #278.
Refs #332.