Repository navigation
Pass the observed choice to the CPN as its last input (#1324) - #1439
AlexanderFengler wants to merge 13 commits into
Conversation
Derived from tests/fixtures/ddm.onnx with LANfactory's derive-aux and trained with the unchanged torchtrain (tiny [32, 32] net, 5 epochs). Width 5: [v, a, z, t, choice] -> log P(choice | theta). Provenance alongside. Co-Authored-By: Claude Fable 5.1 <[email protected]>
The CPN now takes [theta..., extra_fields..., choice] -> log P(choice | theta), the same shape the OPN has with the deadline, so HSSM wires it with params_only=False and assemble_callables hands it the response column of the missing-RT rows. An ONNX CPN/OPN whose input width is not n_params + 1 is rejected at construction with a pointer to #1324. params_only=True stays available for user callables that take only the parameters. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Without a deadline the CPN receives each missing row's response, so those rows must carry a code in model_config.choices; name the offending values. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Re-enable the CPN case of test_make_missing_data_callable (the is_vector=False case stays disabled with its note), pass the response column in the lapse missing-row tests, and hand the width-5 fixture its choice as a fifth scalar where the params-only branches are exercised. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Input layout [theta..., choice] against onnxruntime, the load-time width check on a hand-built (1, 4) graph, the missing-row response validation, and the end-to-end lapse mixture log((1-p) exp(cpn(theta, c)) + p/2) on missing rows of an hssm.HSSM model. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…1324) Co-Authored-By: Claude Fable 5.1 <[email protected]>
…1324) - make_likelihood_callable / make_jax_logp_funcs_from_callable: params-only mode is for callables that take only the parameters; CPN and OPN artifacts take the choice / deadline as their last input - make_missing_data_callable gets its own contract note instead of delegating - LANLogpOp.make_node: data may be the network's single data column; None is the params-only user-callable path only - rename the params-only inner closure (no longer the CPN/OPN path) - type the width-check helper's loglik_missing_data like the constructor and note the second artifact load Co-Authored-By: Claude Fable 5.1 <[email protected]>
…es (#1324) - changelog and reference page describe the corpus layout to retrain on and link the issue for the tooling instead of naming a CLI that is not on any LANfactory release - state precisely where a params-only callable is still accepted (assemble_callables(params_only=True), invoked as f(None, *params)); the hssm.HSSM API always passes the choice - integration test header: deadline selects the data column, both networks use params_only=False Co-Authored-By: Claude Fable 5.1 <[email protected]>
- shared cpn_onnxruntime oracle fixture in conftest (was duplicated) - user callable: compare the per-row mixture on missing rows, not just finiteness, so it pins that the response column reaches the callable - pytensor CPN path: compare against onnxruntime on [theta..., choice] - unit-test _check_missing_data_network_input_width directly (n_params including extra fields, callables, NONE, symbolic dims) - pin the deadline gate of the missing-row response check - set missing rows' responses explicitly instead of asserting on ssms' draws - guard the fixture against its provenance (sha256, shape, sample outputs) - seed test_missing_and_deadline's data fixture Co-Authored-By: Claude Fable 5.1 <[email protected]>
H1 (#1438) is now on main, so the shared #1322 commits drop out. One conflict, a docstring hunk in jax.py's make_jax_logp_funcs_from_callable: keep H2's sentence on the single-column missing-data callable and main's paragraph on the single-trial vmap signature. Co-Authored-By: Claude Opus 5.5 <[email protected]>
H2 goes into the unreleased 0.5.1 with H1 (0.6.0 is kept for the bambi migration): its changelog entry becomes item 18 of 0.5.1 with the migration note under a breaking-changes subsection, and the width error, the reference page and the tests say 0.5.1. Co-Authored-By: Claude Opus 5.5 <[email protected]>
main's hssm.track records each ONNX load under the current network role. The width check loaded the missing-data network before the missing-data role was set, so tracking recorded the CPN as the likelihood network (test_tracking's two missing-data cases). Co-Authored-By: Claude Opus 5.5 <[email protected]>
#1436 took item 18 of the unreleased 0.5.1, so H2's changelog entry becomes item 19; the breaking-changes note stays at the end of 0.5.1. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Route data columns to missing-data networks src/hssm/distribution_utils/dist.py, src/hssm/distribution_utils/jax.py, src/hssm/distribution_utils/onnx.py, src/hssm/hssm.py, tests/distribution_utils/test_jax.py, tests/conftest.py, tests/fixtures/ddm_cpn.provenance.json, tests/integration/test_missing_data_mcmc.py, tests/test_lapse_missing_rows.py, tests/test_make_jax_single_trial_logp_from_networks.py, tests/test_missing_and_deadline.py, tests/test_cpn_choice_input.py |
assemble_callables passes the response column to CPNs and the deadline column to OPNs when params_only=False. HSSM configures CPNs to use this input path. Tests cover CPN evaluation, gradients, and single-trial and vectorized calls. |
Validate network widths and missing-row responses src/hssm/hssm.py, src/hssm/data_validator.py, docs/changelog.md, docs/reference/models-and-likelihoods.md, src/hssm/base.py, tests/test_cpn_choice_input.py |
HSSM rejects CPN or OPN ONNX networks with a static input width other than n_params + 1. When missing data is enabled without a deadline, invalid responses on missing-RT rows raise a ValueError. Documentation describes the input contract and migration behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant HSSM
participant assemble_callables
participant CPN
HSSM->>assemble_callables: Send model parameters and missing-row data
assemble_callables->>CPN: Send parameters followed by observed choice
CPN-->>assemble_callables: Return choice log-probability
assemble_callables-->>HSSM: Return missing-row log-probabilities
Merge Risk: 
Minimal · up to 90d92
-
Missing-row CPNs receive the observed choice. No material merge risk remains from the reviewed changes.
Pre-merge checks: 
5
-
Full details: Title check
-
Explanation
The title "Pass the observed choice to the CPN as its last input (
#1324)" matches the main change in the changeset. The raw summaries show the CPN input contract change: networks receive[θ…, choice]as the final input, the width check, and the response validation on missing-RT rows. The linked objectives describe the same change, and the issue number is referenced. The title is concise, specific, and a single sentence.
Full details: Linked Issues check
-
Explanation
Issue
#1324is directly linked and has coding requirements. The supplied PR evidence shows that HSSM usesparams_only=Falsefor CPNs,assemble_callablesappends each missing-RT row'sresponse, and the validator rejects invalid missing-row responses with a reference to#1324. HSSM rejects fixed-width ONNX CPN and OPN inputs that are notn_params + 1, and the check runs under missing-data tracking. The DDM CPN fixture was regenerated at width 5. The CPN test case was re-enabled, and the reported tests cover response propagation, width validation, fixture behavior, lapse mixing, and gradients. The evidence also states that the1 / n_choiceslapse term is unchanged.
Full details: Out of Scope Changes check
-
Explanation
The supplied whole-PR summary ties the implementation, validation, ONNX fixture and provenance, documentation, changelog, and tests to issue
#1324. The OPN width check shares the documentedn_params + 1contract and supports the common missing-data network path. Theparams_only=Truecallable support documents and preserves the migration compatibility stated in the PR. No unrelated change is demonstrated. Issue#1323is explicitly excluded.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 86.36% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 22 functions across 13 files. (3 skipped: 3 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Spine plan: https://github.com/lnccbrown/HSSMSpine/blob/main/plans/derive-aux-networks.md#pr-stack (step H2 of 8)
Depends on:
main(H1, #1438, is merged)Unblocks: CPNs on the Hub, which are promoted only once a release contains this; the pipeline's CPN
hssm_missing_loadgateCloses #1324
Why
rt == -999.0, no deadline) still has an observed choice, but the CPN was called with the parameters only. Every such row got the probability of one fixed choice, whichever choice the participant made.What
[θ…, choice] -> log P(choice | θ), the same layout the OPN already uses with the deadline ([θ…, deadline]). Both have input widthn_params + 1.HSSMwires the CPN withparams_only=False, like the OPN.assemble_callablessplices in theresponsecolumn for missing-RT rows and the deadline column for omission rows.hssm.track's missing-data role, so a tracked fit still records the CPN as its missing-data network.responseis not inchoicesnow raise a clearValueError. The data validator used to skip these rows.tests/fixtures/ddm_cpn.onnxis regenerated at width 5 from the DDM LAN fixture with LANfactory'sderive-aux. Its provenance is intests/fixtures/ddm_cpn.provenance.json.1 / n_choices).HSSM,HSSMBase,make_missing_data_callable,assemble_callables,make_jax_logp_funcs_from_callable) anddocs/reference/models-and-likelihoods.mddescribe the contract. Changelog: 0.5.1 item 19, plus a "Breaking changes" note.Migration (hand-trained CPNs only)
n_params) is rejected at build time. Retrain it on rows whose last column is the choice,[θ…, choice], labelled withP(choice | θ).franklab/HSSM, so no Hub artifact breaks.assemble_callables(..., params_only=True)still work. They are called asf(None, *params).Not in this PR
Tests
tests/test_cpn_choice_input.py(13 tests): the fixture against its provenance, the width error for a params-only CPN and a wrong-width OPN, invalid responses on missing rows, a user callable receiving the missing rows' response, and the missing-row log-likelihood against the lapse mixture of the CPN computed with onnxruntime.tests/test_missing_and_deadline.py: the CPN case runs again (JAX and PyTensor agree on value and gradient).tests/test_make_jax_single_trial_logp_from_networks.py: the input vector layout is pinned.tests/test_lapse_missing_rows.pyandtests/integration/test_missing_data_mcmc.py: H1's closed-form CPN test and the missing-data MCMC grid run under the new contract.🤖 Generated with Claude Code
Summary by CodeRabbit