Conversation
AlexanderFengler
left a comment
There was a problem hiding this comment.
Thanks @hayden-johnson left few smaller comments but overall looks good already.
|
@hayden-johnson I think you mentioned some small tutorial, was it this PR, maybe just forgot to |
Ah so I added it to PR 840 in this commit, so I could show the pipeline from simulation to parameter recovery. Should I add a notebook here as well? |
AlexanderFengler
left a comment
There was a problem hiding this comment.
Hey @hayden-johnson,
thanks this is basically good, just a few minor adjustments to accommodate the drift in the underlying repo code.
Relevant changes that happened in the design of ssm-simulators in the meanwhile.
theta_processor.pyis now deprecated so we only need to care about themodularversionmodular_theta_processor.pyis renamed tomodular_parameter_simulator_adapter.py.- The model configs now explicitly carry the model wise parameter transforms that are applied: The pattern is that configs now have a "parameter_transforms" dictionary that has "sampling" (for parameter sampling stage, most models don't have any) and "simulation" (for the adaptation of parameters from passing into the python simulator to passing correctly down to the c-level simulator. Here's an example:
def get_race_no_z_2_config():
"""Get configuration for race model with 2 choices and no z."""
return {
"name": "race_no_z_2",
"params": ["v0", "v1", "a", "t"],
"param_bounds": [
[0.0, 0.0, 1.0, 0.0],
[2.5, 2.5, 3.0, 2.0],
],
"boundary_name": "constant",
"boundary": bf.constant,
"n_params": 4,
"default_params": [0.0, 0.0, 2.0, 1e-3],
"nchoices": 2,
"choices": [0, 1],
"n_particles": 2,
"simulator": cssm.race_model,
"parameter_transforms": {
"sampling": [],
"simulation": [
SetZeroArray("z", shape=(None, 2)),
ColumnStackParameters(["v0", "v1"], "v", delete_sources=False),
ExpandDimension(["t", "a"]),
],
},
}
Lastly, if you can, please take the current model out of the race.py file and add a poisson_race.py file instead. I think it's sufficiently different to warrant it's own file even though it's called "race" as well.
Let me know if you have other questions, either way I think it's going to take you only ~10min to work through the conflicts and make the adjustments. This is 99% there.
There was a problem hiding this comment.
This looks good now afaic, thanks @hayden-johnson.
Just run pre-commit locally then should be good to go.
ruff format . seems to be failing on git actions.
This PR provides an initial implementation of the Poisson race model described in Appendix A of:
A comparison of two response time models applied to perceptual matching
Key Differences from the Original Formulation
Stimulus-dependent rate parameters are not implemented. This would require trial-by-trial stimulus identifiers for log-probability evaluation.
See PR 840 in HSSM.