Skip to content

model: store prefixes directly on the model - #1502

Open
yebai wants to merge 7 commits into
bindingsfrom
prefixes
Open

yebai wants to merge 7 commits into
bindingsfrom
prefixes

Conversation

@yebai

@yebai yebai commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

This stores prefixes and their prefix templates on Model, removing PrefixContext and extract_prefixes. A prefix template keeps the parent's storage shape for nested submodels: for a model m with LHS variable x, prefix(m, @varname(a[end]); template=zeros(2)) places x at a[2].x in a two-element parent array. Prefix indices are scalar integers (a scalar CartesianIndex is canonicalised, and begin/end resolve against the template); a slice or range prefix throws, and so does an auto-prefixed submodel on a sliced LHS (a[1:2] ~ to_submodel(m)); use to_submodel(m, false) there. Bindings use absolute prefixed names and must be at or below the model's prefix. DynamicPPL.getprefix(model) returns the combined prefix (nothing when absent). Observation calls take that prefix instead of a context: tilde_observe!!(ctx, dist, value, vn, template, vi) → tilde_observe!!(prefix, dist, value, vn, template, vi). This adopts template propagation as the solution to the prefix and shape problems in #1221.

Closes #1221.

@yebai
yebai added this pull request to stack #1505 September 16, 2026 21:16
@codecov

codecov Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.43750% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.95%. Comparing base (7a01c51) to head (5aff9d6).

Files with missing lines Patch % Lines
src/submodel.jl 87.50% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           bindings    #1502      +/-   ##
============================================
+ Coverage     89.63%   89.95%   +0.32%     
============================================
  Files            52       51       -1     
  Lines          7165     7106      -59     
============================================
- Hits           6422     6392      -30     
+ Misses          743      714      -29     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

DynamicPPL.jl documentation for PR #1502 is available at:
https://TuringLang.github.io/DynamicPPL.jl/previews/PR1502/

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes core compiler and nested-model evaluation semantics, warranting final human validation across downstream integrations.

Pull request overview

Moves prefix and template metadata from PrefixContext into Model, simplifying context handling while preserving nested-submodel storage shapes.

Changes:

  • Adds model-owned prefix/template propagation.
  • Removes PrefixContext and related APIs.
  • Adds prefix, conditioning, fixing, and dynamic-template tests.
File summaries
File Description
HISTORY.md Documents the breaking context removal.
docs/src/api.md Updates context API documentation.
ext/DynamicPPLInputProvenanceExt.jl Preserves prefix metadata when tracing models.
src/DynamicPPL.jl Removes obsolete exports and inclusion.
src/compiler.jl Reads prefix metadata directly from models.
src/contexts.jl Removes prefix-specific context assumptions.
src/contexts/prefix.jl Deletes the former prefix context implementation.
src/model.jl Stores and applies prefixes and templates on Model.
src/submodel.jl Propagates model-owned prefixes through submodels.
test/prefix.jl Tests prefix composition and stored values.
test/runtests.jl Registers the new test suite.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Benchmarks

Performance Ratio: gradient time divided by log-density time.

Primal times are for main (94e4e4a) and this PR (5aff9d6). For very small models the gradient ratios vary across runs and machines, so compare the raw timings instead. The benchmarks are meant for DynamicPPL developers and mainly catch obvious allocation and type-stability regressions; see the benchmark notes for details.

================================================================================================================================
                                                                     primal                            gradient                 
                                                             ----------------------  -------------------------------------------
Model                                         dim    linked        main          PR     FwdDiff    RvsDiff    Mooncake    Enzyme
--------------------------------------------------------------------------------------------------------------------------------
Simple assume observe*                          1     false     2.78 ns     5.56 ns        9.53    1341.14       10.47      6.14
Simple assume observe*                          1      true     2.76 ns     5.56 ns        9.51    1275.81       10.44      6.16
Smorgasbord                                   201     false     3.24 μs     5.32 μs       73.92     149.10        5.74      1.41
Smorgasbord                                   201      true     3.79 μs     6.49 μs       74.65     172.25        5.79      1.53
Smorgasbord fixed                             100     false           —     2.55 μs       50.17     187.67        6.76      1.28
Smorgasbord fixed                             100      true           —      3.0 μs       58.35     206.46        6.86      1.51
Smorgasbord partial condition                 201     false           —     5.72 μs       66.76     136.47        6.38       err
Smorgasbord partial condition                 201      true           —     6.81 μs       77.01     156.45        6.53       err
Smorgasbord partial decondition               202     false           —     7.94 μs       73.23     101.55        4.87       err
Smorgasbord partial decondition               202      true           —     8.95 μs       81.73     123.52        5.23       err
NamedTuple-field LHS variable*                  1     false           —     5.56 ns        9.12    1414.78       10.56      5.84
NamedTuple-field LHS variable*                  1      true           —     5.56 ns        9.15    1406.34       10.58      5.84
MvNormal argument-supplied observation*         1     false           —     83.2 ns        1.92     950.06        3.33      2.19
MvNormal argument-supplied observation*         1      true           —     80.0 ns        2.13     964.25        3.39      2.22
Loop univariate 1k                           1000     false     8.17 μs     16.9 μs      905.01     331.40        4.82      0.82
Loop univariate 1k                           1000      true     10.6 μs     18.1 μs     1351.73     306.04        5.07      0.78
Multivariate 1k                              1000     false    741.0 ns     1.03 μs     1343.98    1014.29        1.61      3.64
Multivariate 1k                              1000      true    791.0 ns      1.0 μs     1420.36    1053.15        1.77      3.71
Loop univariate 10k                         10000     false     77.5 μs    163.0 μs    22964.83     359.80        6.37      0.70
Loop univariate 10k                         10000      true     98.5 μs    176.0 μs    23903.72     336.79        4.87      0.69
Multivariate 10k                            10000     false     5.06 μs      9.1 μs    28673.91    1155.18        1.77      2.84
Multivariate 10k                            10000      true     4.92 μs     9.04 μs    28024.32    1168.24        1.67      2.83
Dynamic                                        15     false           —      1.3 μs         err      44.90       16.63      5.54
Dynamic                                        10      true     1.19 μs     1.79 μs        1.61      65.36       15.01      7.40
Submodel*                                       1     false     2.82 ns     5.57 ns        9.61     867.98       10.55      6.14
Submodel*                                       1      true     2.82 ns     5.57 ns        9.59    1532.81       10.55      6.20
LDA                                             6      true     2.55 μs     3.54 μs        1.49      12.55       27.51     33.23
================================================================================================================================
Environment
Julia Version 1.13.1
Commit 96ca370cf0e (2026-09-25 19:34 UTC)
Build Info:
  Official https://julialang.org release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 4 × AMD EPYC 7763 64-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-20.1.8 (ORCJIT, znver3)
  GC: Built with stock GC
Threads: 1 default, 1 interactive, 1 GC (on 4 virtual cores)

@yebai
yebai force-pushed the prefixes branch 10 times, most recently from 782a8e4 to 5070aa5 Compare October 3, 2026 11:25
Comment thread src/conditionfix.jl Outdated
for name in keys(values.data)
name === AbstractPPL.getsym(prefix) || throw(
for name in (addresses === nothing ? keys(values) : addresses)
AbstractPPL.subsumes(prefix, name) || throw(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a corner case here, where, e.g., name is p[1, 1].x and perfix is p[1]; for matrix and linear indexing, p[1] and p[1, 1] are aliases, but the current code will reject.

yebai added 7 commits October 10, 2026 21:50
Assisted-by: Codex <codex@openai.com>
Repair the stale metadata_site test reference inherited from bindings.

Assisted-by: Codex <codex@openai.com>
Assisted-by: Codex <codex@openai.com>
Assisted-by: Codex <codex@openai.com>
The public observation hook now takes the model's single prefix value,
`tilde_observe!!(prefix, right, left, vn, template, vi)`, instead of a
prefix and a separate prefix template. The model is still not passed,
so observations do not box it; timings and allocations are unchanged.

Assisted-by: Codex <codex@openai.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get rid of PrefixContext & make submodels much better

3 participants