Repository navigation
Add the ddm_normal_st model config (config part of #361, stacked on #349) - #369
Merged
Merged
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds and exports the Changesddm_normal_st model
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to This change adds a new ddm_normal_st model configuration with tests. No actionable merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
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.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
This is the config part of #361 (890c8e9), split off so that the model can be released without #359, which that branch is stacked on and which is not ready. ddm_normal_st draws ndt ~ Normal(t, st), so st is a standard deviation, against the uniform half-width that ddm_st means by the same parameter. Parameters, order, bounds and simulator machinery are ddm_st's; only the kernel differs. The model joins KDE_NO_DISPLACE_T like the other ndt-variability models. The kernel is unbounded, so it emits rt <= 0 by design (about 9% of samples at the box corner t=0.25, st=0.25, a=0.3). KDE label generation therefore needs the non-positive-RT filter of #349, which this commit sits on. Left out of 890c8e9: - The Normal branch of _recover_admissibility_boundary in kde_class.py and its three tests. The helper only exists after #359, so they belong with that PR. - The ddm_uniform_st alias of ddm_st (config function, registry entry, KDE_NO_DISPLACE_T entry and test). The name of the uniform model is being settled separately, and an alias can be added later without breaking anything. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
At t=0.25, st=0.25, a=0.3 about 9% of ddm_normal_st's RTs are non-positive. Without the non-positive-RT filter of #349, one such RT turns its whole choice group into "no_base_data" and its labels into a constant, on the displace_t=False path the generator takes for this model. The test fails when kde_class.py is restored to main's version, so it pins the model's dependency on that filter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
AlexanderFengler
force-pushed
the
feat/ddm-normal-st-config
branch
from
October 7, 2026 13:19
05edf65 to
9092013
Compare
This was referenced Oct 7, 2026
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.
Config part of #361 (890c8e9), split from #359 so that
ddm_normal_stcan go into the next release (lnccbrown/HSSM#1345 needs it) without waiting for #359.Stacked on #349. Until #349 merges, the diff also shows its commits; afterwards it shrinks to the two commits below. Merge #349 first, with a merge commit.
What it adds
ddm_normal_st: ndt ~ Normal(t, st), sostis a standard deviation (ddm_st'sstis a uniform half-width). Same parameters, order, bounds and simulator machinery asddm_st; only the kernel differs._modelconfig/__init__.pyand aKDE_NO_DISPLACE_Tentry, so the generator forcesdisplace_toff as for the other ndt-variability models.tests/test_ddm_normal_st_config.py: the config tests from Feat/ddm normal st model config #361, plus one LogKDE test at the box corner (t=0.25, st=0.25, a=0.3, about 9% non-positive RTs) that fails without Filter non-positive RTs before KDE bandwidth estimation #349's filter.Left out of 890c8e9, on purpose
_recover_admissibility_boundaryinkde_class.pyand its three tests: the helper only exists after Displace RTs by the true admissibility boundary t - st, not by t #359 and belongs with it.ddm_uniform_stalias (config, registry entry,KDE_NO_DISPLACE_Tentry, test): the naming of the uniform model is being settled together with HSSM, and an alias can be added later without breaking anything.Why #349 underneath
no_base_datawith flat labels, on the default generator path.Evidence
model_config["ddm_normal_st"]hasddm_st's params and bounds;ddm_uniform_stexists nowhere (git grep).ddm_stcontrol); the LogKDE test fails withkde_class.pyatmain(both bandwidthsno_base_data, labels flat) and passes here.TrainingDataGeneratorruns (default box, and the box narrowed to the corner so every parameter set has rt <= 0): all labels finite, no choice group collapsed,displace_t=Trueforced toFalsewith the warning.--run-statistical: 1284 passed, 94 skipped.ruff checkandruff format --checkclean.mainand with Displace RTs by the true admissibility boundary t - st, not by t #359, Give each generated file its own theta block and pin the sampling order #363, Bind trial-to-trial variability draws to a seeded Generator instead of the global RNG #348 and Feat/kde choice proportions override #362 on top.Relation to #361: this supersedes its config part; the boundary-helper part of #361 stays with #359.
🤖 Generated with Claude Code
Summary by CodeRabbit
ddm_normal_stmodel option, which samples non-decision time from a Normal distribution and is available alongside the existing model configurations.st.