fix: guard a leaf whose parameters are vectors, not scalars - #370
Closed
seabbs-bot wants to merge 1 commit into
Closed
fix: guard a leaf whose parameters are vectors, not scalars#370seabbs-bot wants to merge 1 commit into
seabbs-bot wants to merge 1 commit into
Conversation
A leaf with vector-valued parameters, Categorical being the motivating case, silently emitted a Vector into a scalar parameter row. The table walk defines one row per scalar parameter, so there is no correct row for a probability vector. The guard existed only in Uncertain's constructor, so a fixed leaf reached the plain walk unchecked. Throw there too, naming the leaf and pointing the caller at direct parameter access. Composing and scoring such a leaf is unaffected and stays supported; only the table view is undefined for it. Salvaged from an uncommitted branch left by an earlier session. That branch also changed leaf_mean/leaf_var to stop reading through free_leaf, which is a real defect, but PR #365 fixes it better: it falls back to the free-leaf approximation when Distributions.jl has no closed form, where the unconditional version throws for Truncated{Gamma}. That half is dropped here in favour of #365.
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/vector-param-guard-and-truncated-moments")
using ComposedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout fix/vector-param-guard-and-truncated-moments
julia --project=. -e "using Pkg; Pkg.instantiate()" |
This was referenced Aug 10, 2026
Collaborator
Author
|
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. |
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
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.
A leaf with vector-valued parameters silently emitted a
Vectorinto a scalar parameter row. The table walk defines one row per scalar parameter, so there is no correct row for a probability vector.The guard existed only in
Uncertain's constructor, so a fixed leaf reached the plain walk unchecked.Categoricalis the motivating case.What changes
Six lines in
_walk_rows!(src/composers/introspection.jl) that throw when a leaf'sparamsare not allReal, naming the leaf and pointing the caller at direct parameter access.Composing and scoring such a leaf is unaffected and stays supported — only the table view is undefined for it. The test pins both halves:
logpdfstill works, the table throws.Provenance, and one half deliberately dropped
Salvaged from a branch an earlier session left uncommitted. That branch also changed
leaf_mean/leaf_varto stop reading throughfree_leaf, so aTruncatedleaf stops reporting its untruncated moment. That is a real defect, but #365 fixes it better: it falls back to the free-leaf approximation where Distributions.jl has no closed form, whereas the unconditional version throws aMethodErrorforTruncated{Gamma}. That half is dropped here in favour of #365, so the two PRs do not collide.Gates
Targeted verification only, run against this branch: the guard throws for a
Categoricalleaf,logpdfon the same tree still returns aReal, and an ordinary two-leaf tree still produces its four parameter rows unchanged. The full suite was not run — the machine is saturated with the other agents in this batch, and CI will run it here anyway.Rebase note
Touches the table walk that #343 rewrites, and uses
params_table, which #343 removes in favour ofcomposed_to_table. Whichever merges second needs a small rename in the guard's test. Kept separate from #365 for exactly this reason.This was opened by a bot. Please ping @seabbs for any questions.