refactor!: drop the LogDensityProblems/DynamicPPL/as_turing ext + ConvolvedDistributions re-export#238
Closed
seabbs-bot wants to merge 1 commit into
Closed
Conversation
…volvedDistributions re-export (#220, #233, #228) DistributionsInference.jl now hosts the LogDensityProblems interface (hard dependency there) and the generic as_turing model over the same fit protocol, superseding ComposedDistributions' own weakdep extensions (#220, #233); CD's own PPL-neutral ComposedLogDensity/ as_logdensity/logdensity core is unchanged. test/composers/turing_ext.jl is ported to call DistributionsInference.as_turing rather than deleted, since it is the live regression guard for the params_table/codec ordering coupling (#192) -- verified DI's dotted VarName naming is byte-identical to the removed CD-native stub, so a chain sampled from DI's as_turing still round-trips through CD's own untouched chain_to_params/update(tree, chain). Also drops the ConvolvedDistributions re-export (#228): 14 ConvolvedDistributions-owned names were part of CD's advertised API purely by re-export, so every upstream change was silently a CD API change too. CD's own composer code keeps its internal, unexported using of the 8 names it actually calls; callers reach ConvolvedDistributions' own surface with an explicit `using ConvolvedDistributions`. Co-authored-by: Sam Abbott <contact@samabbott.co.uk>
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="refactor/drop-redundant-inference-glue-220-233-228")
using ComposedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout refactor/drop-redundant-inference-glue-220-233-228
julia --project=. -e "using Pkg; Pkg.instantiate()" |
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 — 24 benchmarks (by time change)
|
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:
|
4 tasks
Collaborator
Author
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
ext/ComposedDistributionsLogDensityProblemsExt.jland theLogDensityProblemsweakdep.DistributionsInference.jl'sFitLogDensitynow implements theLogDensityProblemsinterface directly (a hard dependency there), superseding this thin weakdep wrapper around CD's ownComposedLogDensity.src/composers/turing.jl(theas_turingstub) andext/ComposedDistributionsDynamicPPLExt.jl.DistributionsInference.as_turingis the generic replacement (its own docstring: "ported from ComposedDistributions'as_turing... which this extension supersedes"). Verified its dottedVarNamenaming is byte-identical to the removed CD-native stub's, so a chain sampled fromDistributionsInference.as_turingstill round-trips through CD's own untouchedchain_to_params/update(tree, chain)(ext/ComposedDistributionsFlexiChainsExt.jl, out of scope here — tracked on Lots of packages have update syntax do we want to be careful here and not export us but make it public? #221).ConvolvedDistributionsre-export (14 names:convolved,convolve_series,discretise_pmf,DelayPMF,Difference,difference,product,Product,AnalyticalSolver,NumericSolver,AbstractSolverMethod,GaussLegendre,integrate,gl_integrate). CD's own composer code keeps an internal, unexportedusing ConvolvedDistributions: ...for the 8 names it actually calls; a caller reaching ConvolvedDistributions' own surface now does its ownusing ConvolvedDistributions.CD's own PPL-neutral
ComposedLogDensity/as_logdensity/logdensity/to_constrainedcore (src/composers/logdensity.jl) is unchanged — only theLogDensityProblems/DynamicPPL-facing wrappers around it move to DI.Notable choices
test/composers/turing_ext.jlis ported, not deleted: it is the live regression guard for the params_table/codec ordering coupling (Unify params_table onto the generated codec walk + add the shared-tag/as_turing ordering guard test (#178 follow-up) #192), exercised via a real NUTS round trip. It now callsDistributionsInference.as_turing, added as a git-pinned test dep (test/Project.toml, matching the existingModifiedDistributions/EpiAwarePackageToolspattern for unregistered EpiAware packages)."re-exported ConvolvedDistributions surface is reachable","product / Product reachable via ComposedDistributions (#139)") are deleted rather than patched.docs/src/getting-started/inference.mdandfaq.mdare rewritten to point external-sampler/Turing consumption atDistributionsInference, keeping CD's own PPL-neutral density andto_constrainedsections unchanged.Test plan
julia --project=. -e 'using Pkg; Pkg.test(test_args=["skip_quality"])'— green (full suite, excluding the QA-only testset and the separately-CI'd:ad-tagged items).Base.get_extensionchecks) andas_turing/product/etc. are genuinely gone fromComposedDistributions, whilechain_to_params/convolved(unexported) remain.This was opened by a bot. Please ping @seabbs for any questions.