Per-element parameterization: mix and match constraints per instance (8.2.1) - #184
Merged
Conversation
`Parameter.build_pymc` set `is_derived` for a WHOLE vector
(`np.full(n_elements, expr_raw is not None)`), so every modeling choice that
is really per instance had to be uniform across a component. Four shipped
features paid for that: `band.ld_law` RAISED on a system mixing quadratic and
linear bands, `planet.mass_parameterization` raised on explicit disagreement
and silently fell back to all-linear on an implicit one, `star.mist`'s
declared `mask` was never read (so a premature `evolutionarymodel:` block
materialized three free, likelihood-free dimensions -- review 3.8.2), and
`orbit.fitvcve`'s WIP guard named the unconsumed `mask` field as its real
blocker, ahead of the missing physics.
This is the primitive those four will consume; no component changes yet.
Four element roles, interpreted in manifest.py and consumed in build_pymc:
sampled a raw coordinate of its own (the default, and every element of
every parameter before this)
derived an expression the model CONSUMES
reported an expression nothing consumes -- the reverse direction of a flip
(report sqrt(e)cos(omega) for an orbit that sampled V_c/V_e).
Vocabulary and errors ship here; its deferred build pass lands
with its first consumer, vcve (review 8.8.3).
inactive not a parameter of that instance at all (manifest `mask`): held at
a bookkeeping value, given no potential, and reported NOWHERE,
because a value nothing reads is at best meaningless and at worst
read as a result.
Vocabulary: `expr_key` may be `{block: selector}` per element,
`output_expr_key` the same for role 3, `mask` is the activity selector, and
`inactive_value` states what a masked-out element is held at (a linear-law
band's u2 is exactly 0, not an unrelated default). One selector normalizer,
sized from the PARAMETER's element count -- never the component's config list,
which is review 1.1.1's hazard in a new place, so a length mismatch raises.
Notes worth keeping:
* The mixed assembly uses `pt.set_subtensor`, never a `pt.where` over the two
value vectors. An expression evaluated at an unused element's pin can
legitimately be NaN (sqrt of a negative eccentricity the other
parameterization never promised), and where's VJP multiplies the unselected
branch by zero -- 0*NaN poisons the gradient of the whole vector on every
backend.
* Better still, the unused elements never enter the expression:
`Component._element_expression` slices the deps to the mask, but only where
the alignment is PROVABLE (a local parameter of equal length, a map with one
entry per element, or a context node the component lists in
`aligned_context_deps`). A bare cross-component vector is indexed by the
OTHER component's elements and raises -- the same call this function already
makes for a dep naming a map the component does not have.
* "Elementwise" is a property of the FUNCTION and cannot be proven statically,
so both graphs are kept and compared at the START POINT
(`System.verify_element_slices`, one compile at the end of build_model, no-op
without a mixed vector). Dummy inputs could agree by accident, and
evaluating a random variable would draw from its prior instead of reading the
start.
* A parameterization flip carries almost every constraint for free, which is
what lets one params file toggle it: `mu`/`sigma` on an element that became
derived still applies, an `initval` is still a RANK_USER assignment the
engine propagates into whatever is sampled, and only `lower`/`upper` change
meaning (hard logit support -> soft barrier). `inactive` is the one lossy
case and warns per element, keyed on what the USER wrote -- nearly every
parameter has bounds from defaults.yaml, so keying on resolved values would
warn about everything.
* Reporting is per element throughout (element_is_derived/_reported/_active;
get_prior_str reads the element). Inactive elements are suppressed from the
startup table, the LaTeX table and results.csv -- an instance sub-head only
if the instance has rows -- while their macros are still DEFINED: an uncited
definition is harmless, a cited-but-undefined one is an "Undefined control
sequence" at the end of a long fit.
* `System.derived_elements()`/`active_elements()` are the per-element answers
solve_api, `export_solution` (new "active" field) and the GUI's Tune tab now
use; `derived_params()` keeps its whole-vector meaning, which is the
conservative direction because its consumers treat "derived" as "exempt".
* mkparam has no System and a raw variable's length says how MANY elements are
sampled, never which -- so `trace_meta.element_roles` stamps the non-uniform
masks into the trace attrs and mkparam filters on them. Absent on older
traces and on uniform models, where "every element sampled" is what a
missing entry always meant.
Batched in: review 1.2.3 -- to_latex_def's fixed path now sizes its macro loop
from `shape` (broadcasting the initval) instead of `len(atleast_1d(initval))`,
as the adjacent \nodata branch already did, so a pinned vector with a broadcast
scalar initval no longer emits one macro for a body that cites several.
Verification: all 19 shipped example configs are BIT-IDENTICAL -- start logp
(compared as the exact float repr), free-RV set, raw-coordinate count, and
every parameter's resolved values/bounds/sigma/mu -- and the full suite passes
(2329). The JAX path is exercised by really sampling a mixed model with
nuts_sampler="numpyro", per the standing rule that a finite C-backend gradient
is not proof.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t-ins
Two helpers in a new `components/parameterization.py`, on top of the
per-element roles:
`mode_manifest(modes, table, options=...)` -- a component holds a per-instance
CHOICE read from its own config (`ld_law` per band, `mass_parameterization` per
planet, `mist`/`parsec` per star, `fitvcve` per orbit) plus a statement of what
each choice uses; turning that into masks is mechanical, and writing it out per
component is how four implementations of one idea drift apart. A parameter a
mode does not name is INACTIVE on that mode's elements -- that is how a linear
limb-darkening law says it has no `q2`.
Two properties make it safe to adopt everywhere:
* a SINGLE-mode system expands to exactly the manifest the component used to
hand-write (a bare-string `expr_key`, no `mask`, and an `inactive_value`
that could not do anything dropped), so those systems build an identical
graph;
* a parameter NO instance uses is omitted entirely rather than declared
wholly inactive -- an all-linear band set has no `u2` at all, which is what
its consumers' `"u2" in band.manifest` guard reads.
`pin_unselected(n_elements, selected)` -- the OPT-IN pin, and the difference
from an inactive element is why both exist. It pins through `"overrides"`,
which layers UNDER the params file, for a parameter that exists for every
instance but is only wanted on some (a GP hyperparameter on the files that
asked for one, an LD coefficient on the bands something reads, the BEER terms
on the bands that fit them), so a user who explicitly wants one back still
wins. An inactive element's pin is structural and unreported, because freeing
it would add a dimension no likelihood term reads.
`Instrument._register_gp` and `Instrument._register_robust` carried that loop
line for line and now call it (review 4.5.2; Band's two copies follow in the
next commit, which also closes 4.5.3).
Also fixed here, found by the first real consumer: `verify_element_slices`
compiled its check with the default inputs (whatever the outputs need), and a
point carries EVERY value variable, so the call died with "Too many parameter
passed to pytensor function" on any model bigger than the unit test. It now
compiles against `model.value_vars`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A system mixing `ld_law: quadratic` and `ld_law: linear` bands RAISED ("all
bands must use the same ld_law"), and before that error existed
`any(law != "linear")` chose the quadratic manifest for everyone -- silently
giving a band the user declared linear a free `u2` and modelling it as
quadratic. The documented workaround was quadratic everywhere with the linear
bands' `q2` pinned at 0.5, which reproduces `u2 = sqrt(q1)(1 - 2 q2) = 0` but
samples uniformly in `q1` rather than in `u1`.
The reason was never limb darkening: `Parameter.build_pymc` derived a WHOLE
vector or none of it. With per-element roles the configuration is simply
expressed, as a mode table (`Band.LD_MODE_TABLE` -> `mode_manifest`):
quadratic samples the Kipping pair, derives (u1, u2) from it
linear samples u1 itself; has no Kipping coordinates and no u2
So in a mixed system `u1` is derived on the quadratic bands and sampled on the
linear ones, while `q1`/`q2`/`u2` are INACTIVE on a linear band -- not
parameters of it: pinned (u2 at exactly 0, stated through `inactive_value`
rather than inherited from an unrelated default), carrying no potential, and
printing no table row.
Which coordinate the unread-band autopin pins is per band for the same reason:
a `sigma` on a derived element is a silent no-op, so pinning `u1` on a
quadratic band would pin nothing while leaving a linear band's only free
parameter loose. `Band.LD_SAMPLED_PARAMS` names what each law samples, and the
autopin now intersects that with the unread bands per parameter (it also picks
up `pin_unselected`, retiring Band's two copies of that loop -- review 4.5.3).
`"u2" in band.manifest` stays the consumer guard (transit.py x2, rm.py): that
case is now "every band is linear", where `mode_manifest` omits the parameter
entirely. rm.py's comment about it is corrected for the mixed case.
Verification: end-to-end on two transits in two bands with different laws --
both coordinate sets sampled on their own band, u2 exactly 0 on the linear one,
finite logp and gradient at the start
(`test_two_transits_may_use_different_limb_darkening_laws`). All 19 shipped
example configs remain BIT-IDENTICAL, including the all-linear microlensing
ones (DC2018_128, KMT-2019-BLG-1806, ob140939, ob161003) that exercise the
omit-u2 path. test_band.py's mixed-law test now pins the new behavior; its old
premise ("per-element derivation is not expressible in the manifest") is false.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three remaining consumers of the primitive, plus the docs that described
its absence.
planet -- the mass coordinate is per PLANET (`Planet.MASS_MODE_TABLE`). A
`log_q` planet derives its mass from log10(m_p/m_host); a `linear` one samples
the signed mass itself; `log_q` is not a parameter of a linear planet at all,
and vice versa. Both directions used to be impossible: an explicit per-planet
disagreement RAISED ("All planets must share one 'mass_parameterization'") and a
mixed default silently fell back to all-linear, which handed the unconstrained
planet a uniform prior over [-1000, 260000] Mjup instead of the ratio
coordinate its topology asks for. A microlensing lens body cannot take a
linear mass at all (the magnification clips q to [1e-9, 100]), so in a system
with one lens planet and one RV planet the old code had to give one of them the
wrong coordinate.
`Planet.mass_parameterizations` is the per-planet list; `mass_parameterization`
survives as the whole-component answer ("log_q" only when every planet agrees)
for readers that just want to know whether the ratio coordinate is in play.
`_reconcile_mass_user_params` is per planet too -- a stale `planet.<i>.log_q`
entry is only stale for planets that sample a linear mass. Neither soft-bound
potential needed changing: both read `self.mass.value`, which is the mass in
either coordinate.
star -- the `mist:`/`parsec:` mask on `age`/`initfeh`/`eep` (declared since
PR #158, never read) now does what it says: a star with no evolutionary track
has no EEP -- inactive, so nothing samples it, no potential touches it, and no
table reports it. Where NO star opts in, the three are not declared at all.
And since the branch keys on the config KEY, it fires for a premature
`evolutionarymodel:` block that no component backs (review 3.8.2, whose real
edge was exactly those free likelihood-free dimensions): that case now warns,
naming the opted-in stars, because the unrecognized-key warning System emits
does not say what it costs.
orbit -- no code change beyond the guard's own story: `fitvcve: true` still
raises, because `calc_ecc_from_vcve`/`calc_omega_from_vcve` do not exist. What
changes is that the message and docstring no longer name the manifest `mask`
field as "the real blocker, ahead of the missing physics" -- that half is done.
They now name what remains: the physics, and the deferred build pass a role-3
`output_expr_key` needs so a vcve orbit can still REPORT sqrt(e)cos(omega).
That is not polish: an inactive element's prior is dropped (with a warning), so
without the reverse flip, toggling `fitvcve` would silently discard a user's
prior on secosw/sesinw -- the opposite of the point of a per-orbit switch.
Deliberately not writing the vcve mode table here: it would be dead code
naming undefined functions, and the guard is the feature until 8.8.3 lands.
Verification: end-to-end on a two-planet system mixing log_q and linear (both
coordinates sampled, one element each, finite logp and gradient), plus per-star
inactivity end to end for the star mask. All 19 shipped examples remain
bit-identical. Three tests that encoded the old restrictions now pin the new
behavior (`test_a_mixed_topology_gives_each_planet_its_own_coordinate`,
`test_an_explicit_per_planet_mix_is_honored`, and the fitvcve message test).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-element roles reach a trace, so mkparam -- which has no System and cannot otherwise tell WHICH elements of a partially derived vector are sampled (a raw variable's length says how many, not which) -- writes a start value for the linear band's u1 and none for the quadratic band's, whose value is an expression. Asserted inside the mixed-law test rather than a test of its own, since the model it needs is already built there. Verified separately end to end through stamp_structural_metadata on the same topology: band.q1/q2 active+sampled on the quadratic band only, band.u1 derived on element 0 and sampled on element 1, band.u2 active+derived on element 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Mix and match parameterizations per instance:
Parameter.build_pymcno longer derives a whole vector or none of it. Review item 8.2.1.Why
Four shipped features wanted a per-instance choice and could not have one, because
is_derived = np.full(n_elements, expr_raw is not None):band.ld_lawraised on a system mixing quadratic and linear bands (workaround: quadratic everywhere withq2pinned at 0.5, at the cost of a prior uniform inq1rather thanu1);planet.mass_parameterizationraised on an explicit per-planet disagreement and silently fell back to all-linearon an implicit one -- handing an unconstrained planet a uniform prior over[-1000, 260000]Mjup instead of the ratio coordinate its topology asks for;star.mist/parsecdeclared amaskonage/initfeh/eepthat was never read, so a prematureevolutionarymodel:block materialized three free, likelihood-free dimensions (review 3.8.2);orbit.fitvcve's WIP guard named the unconsumedmaskfield as its real blocker, ahead of the missing physics.The four element roles
sampledderivedreportedinactiveVocabulary, all on the manifest entry:
expr_keymay be{block: selector}per element;output_expr_keythe same for role 3;maskis the activity selector;inactive_valuestates what a masked-out element is held at (a linear-law band'su2is exactly 0). One selector normalizer, sized from the parameter's element count -- never the config list (review 1.1.1's hazard) -- raising on a mismatch.Role 3 is declared, not built. Reporting
secoswon a V_c/V_e orbit is derived-but-consumed-by-nothing:eccreadssecoswon the sqrt(e)cos/sin orbits whilesecoswreadseccon the V_c/V_e ones -- acyclic per element, a cycle in the per-parameter ordergraph.pysorts.add_parameterraises aNotImplementedErrornaming the deferred pass that fixes it, and it lands with its first consumer, the vcve parameterization (8.8.3). It matters because an inactive element's prior is dropped: a coordinate with a computable inverse must flip toreportedso a user's prior survives toggling the switch.Notes worth reading in the diff
pt.set_subtensor, never apt.whereover the two value vectors -- an unused element's pin can legitimately make the expression NaN, and0*NaNpoisons the gradient of the whole vector on every backend.add_parameteralready makes for a dep naming a map it does not have.System.verify_element_slices): dummy inputs could agree by accident, and evaluating a random variable would draw from its prior instead of reading the start.mu/sigmastill apply to a now-derived element, aninitvalis still a RANK_USER assignment the engine propagates -- and onlylower/upperchange meaning.inactiveis the one lossy case and warns per element, keyed on what the user wrote.components/parameterization.pymakes declaring a parameterization a table (mode_manifest), withpin_unselectedas the separate opt-in pin (through"overrides", so a user who wants the parameter back still wins) -- retiring the three byte-identical pin loops (reviews 4.5.2, 4.5.3).Verification
nuts_sampler="numpyro"(a finite C-backend gradient is not proof).results.csv, the compiled paper draft, plots and the restart file.Also fixed: review 1.2.3 (a pinned vector with a broadcast scalar initval emitted one macro for a body citing several -- an "Undefined control sequence" at the end of a long fit). Closes 3.8.2, 4.5.2, 4.5.3; partially addresses 2.1.4.
🤖 Generated with Claude Code