tests: add xfail tests for float lapse_dist bugs in _apply_lapse_model#962
Open
cpaniaguam wants to merge 1 commit into
Open
Conversation
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.
@AlexanderFengler @digicosmos86 Would it be sensible to address this?
Summary
Adds two
xfail(strict=True)tests that document two latent bugs in_apply_lapse_model(in dist.py). The tests are currently expected to fail and will flip to passing once a fix is added.Background
For choice-only models (e.g.
softmax_inv_temperature_2),_check_lapsesetsself.lapse = 1 / n_choices— a plainfloat. This float is stored asHSSMRV._lapseand passed to_apply_lapse_model(lapse_dist=...)during prior/posterior predictive sampling. The existing code assumeslapse_distis always abmb.Priorobject, so two things go wrong.Bug 1 —
AttributeErroronlapse_dist.args_apply_lapse_modelalways executes:When
lapse_distis0.5(afloat),lapse_dist.argsraises:Trigger: any call to
model.sample_prior_predictive()ormodel.sample_posterior_predictive()on a choice-only model that hasp_outlier != 0.Bug 2 — 2-column replace mask for a 1-column output
Even if Bug 1 were somehow bypassed, the replacement mask is always built as:
Choice-only simulation output has only 1 column (response; no RT). The 2-column mask either silently corrupts the array via
np.putmaskbroadcasting or raises a shape error depending on input dimensions.Trigger: same as Bug 1.
Current status
Both bugs are latent on the
add-choice-only-rlssm-model-to-registrybranch. For RLSSM modelsssms_rng_fnraises before reaching_apply_lapse_model(ssm_simulators does not support2AB_RescorlaWagner_Softmax), so neither bug is reachable through the RLSSM path today. They would be reachable for any other choice-only model supported by ssm_simulators (e.g.softmax_inv_temperature_2) the moment a user passesp_outlier != 0.Fix (tracked separately)
Add an
isinstance(lapse_dist, float)branch in_apply_lapse_modelthat:choices(n_obs,)and 2-D(n_obs, 1)sims_outThe type annotation of
make_hssm_rv'slapseparameter should also be updated frombmb.Prior | Nonetobmb.Prior | float | None.