initial poisson race implementation by hayden-johnson · Pull Request #840 · lnccbrown/HSSM

hayden-johnson · 2025-10-28T13:52:09Z

Issue #801

This PR introduces a preliminary implementation of the Poisson race model as 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.

Shape parameter generalization: we do not restrict the shape parameter to be an integer. This simplifies the implementation but requires evaluation of the incomplete gamma function using scipy, rather than using factorials.

codecov · 2025-10-28T14:38:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/hssm/_types.py	`100.00% <ø> (ø)`
src/hssm/likelihoods/__init__.py	`100.00% <ø> (ø)`
src/hssm/likelihoods/analytical.py	`100.00% <100.00%> (ø)`
src/hssm/modelconfig/poisson_race_config.py	`100.00% <100.00%> (ø)`
tests/test_likelihoods_poisson_race.py	`100.00% <100.00%> (ø)`

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

AlexanderFengler

Thanks @hayden-johnson this looks reasonable as far as the likelihood is concerned. We might want to check in on the trial-wise covariate issues if it remains unresolved.

Apart from that a few more high level comments to get the PR over the finish line.

We need some tests for the basic likelihood
2 We need the basic simulator implemented in ssm-simulators (separate small PR)
We need a small notebook to that illustrates the model and shows a model fit (goal is to start having these for all incoming models and backfill for existing ones, so people have a much easier time getting starting with using specific models in the toolbox).

src/hssm/likelihoods/analytical.py

Copilot

Pull Request Overview

This PR implements the Poisson race model for two-choice response time modeling, based on a comparison of response time models. The implementation uses continuous shape parameters and evaluates log-likelihoods analytically using gamma distributions.

Key changes:

Adds analytical log-likelihood computation for the Poisson race model with two accumulators
Includes comprehensive unit tests validating vectorization, exponential edge cases, and parameter validation
Registers the new model in HSSM's type system and likelihood exports

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/hssm/likelihoods/analytical.py`	Core implementation of `logp_poisson_race` function with parameter bounds and distribution registration
`src/hssm/modelconfig/poisson_race_config.py`	Configuration defining model parameters, priors, and choices for the Poisson race model
`tests/test_likelihoods_poisson_race.py`	Unit tests covering vectorization, exponential special case, parameter validation, and edge cases
`src/hssm/likelihoods/__init__.py`	Exports the new `logp_poisson_race` function
`src/hssm/_types.py`	Registers "poisson_race" as a valid model type

Comments suppressed due to low confidence (2)

src/hssm/likelihoods/analytical.py:1

Corrected grammar: 'We do not condition on the underlying stimulus condition.'

"""pytensor implementation of the Wiener First Passage Time Distribution.

src/hssm/likelihoods/analytical.py:1

The documentation states 'Response > 0 indicates accumulator 1', but the code at line 618 uses r_c = pt.switch(flip, r2, r1) where flip = response > 0. This means positive responses select r2/k2 (accumulator 2), not accumulator 1. The documentation should be corrected to match the implementation, or clarify the accumulator indexing convention.

"""pytensor implementation of the Wiener First Passage Time Distribution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/hssm/likelihoods/analytical.py

cpaniaguam

The tests look great! I added a suggestion for adopting the more declarative and pythonic list comprehension pattern with a function to do the number crunching.

tests/test_likelihoods_poisson_race.py

review-notebook-app · 2025-12-16T02:00:06Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

hayden-johnson · 2025-12-16T02:06:35Z

@AlexanderFengler

Added an implementation of the Poisson race model in ssm-simulators (see PR 246), along with a minimal example notebook in HSSM/docs/poisson_race.ipynb.

Lmk what you think! Feel free to suggest additions to the tutorial notebook.

fix shape bounds Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

add pythonic syntax Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

cpaniaguam

Looking real good! Left a few more observations. I haven't looked at the notebook yet.

cpaniaguam · 2026-01-08T18:36:21Z

src/hssm/likelihoods/analytical.py

+    Each accumulator time follows a Gamma distribution (with continuous
+    shape parameter) with shape parameter k and rate r. The per-trial
+    likelihood decomposes into the density of the winning accumulator
+    evaluated at the observed decision time and the survival function of
+    the losing accumulator at the same time.


Would it be okay to update the first sentence from Each accumulator time follows a Gamma distribution (with continuous shape parameter) with shape parameter k and rate r. to Each accumulator time follows a Gamma distribution with continuous shape parameter k and rate r.?

cpaniaguam · 2026-01-08T18:41:56Z

src/hssm/likelihoods/analytical.py

+    pytensor.tensor.TensorVariable
+        Per-trial log-likelihoods compatible with PyTensor graphs.


The return type in the function's signature says it's np.ndarray. Consider revising this.

Resolved, and added some more detail in the docstring.

cpaniaguam · 2026-01-08T18:46:40Z

src/hssm/likelihoods/analytical.py

+    k_l = pt.switch(flip, k1, k2)
+
+    log_pdf = (
+        k_c * pt.log(pt.maximum(r_c, epsilon))


Why not define r_c_safe as well?

I was originally only using the safe option for problematic computations (i.e., logarithms). For other computations, I was using unsafe variables; in either case, the evaluation is invalid and should be caught by the parameter checks.

I was inconsistent with this, however, in that I used rt_safe outside of the log function later in the pdf - I changed this now.

cpaniaguam · 2026-01-08T18:51:33Z

src/hssm/likelihoods/analytical.py

+    log_pdf = (
+        k_c * pt.log(pt.maximum(r_c, epsilon))
+        + (k_c - 1.0) * pt.log(rt_safe)
+        - r_c * rt_safe


Just curious: why use safe versions for a standard product for one variable but not the other? It seems like it should be fine to use the untreated versions.

True. I meant to only use safe values inside the log function. I changed this for consistency.

src/hssm/likelihoods/analytical.py

cpaniaguam · 2026-01-08T19:12:39Z

docs/tutorials/poisson_race.ipynb

Notebooks looks good to me!

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T14:05:26Z

docs/tutorials/poisson_race.ipynb

+    "    \"r1\": ssms_params[\"r1\"],\n",
+    "    \"r2\": ssms_params[\"r0\"],\n",
+    "    \"k1\": ssms_params[\"k1\"],\n",
+    "    \"k2\": ssms_params[\"k0\"],\n",


The notebook creates a mapping between ssms_params (with r0/r1/k0/k1 naming) and true_params (with r1/r2/k1/k2 naming), where ssms r1 maps to HSSM r1 and ssms r0 maps to HSSM r2. However, there's an inconsistency: ssms k1 maps to HSSM k1, but ssms k0 maps to HSSM k2. This creates confusion because r1 from ssms goes to r1 in HSSM, but k1 from ssms goes to k1 in HSSM while k0 goes to k2. For clarity and consistency, the mapping should be: ssms r0->HSSM r1, ssms r1->HSSM r2, ssms k0->HSSM k1, ssms k1->HSSM k2 (consistently mapping lower accumulator to 1 and upper to 2, or vice versa).

Suggested change

" \"r1\": ssms_params[\"r1\"],\n",

" \"r2\": ssms_params[\"r0\"],\n",

" \"k1\": ssms_params[\"k1\"],\n",

" \"k2\": ssms_params[\"k0\"],\n",

" \"r1\": ssms_params[\"r0\"],\n",

" \"r2\": ssms_params[\"r1\"],\n",

" \"k1\": ssms_params[\"k0\"],\n",

" \"k2\": ssms_params[\"k1\"],\n",

Copilot · 2026-01-28T14:05:27Z

src/hssm/likelihoods/__init__.py

    "logp_ddm_bbox",
    "logp_ddm_sdv_bbox",
    "logp_full_ddm",
+    "logp_poisson_race",


The POISSON_RACE distribution object is created but not exported in the init.py file. The init.py only exports logp_poisson_race function but not the POISSON_RACE distribution object itself. Looking at similar models like LBA2 and LBA3, both the logp function and the distribution object should be exported for consistency.

src/hssm/likelihoods/analytical.py

Copilot · 2026-01-28T14:05:28Z

src/hssm/likelihoods/analytical.py

+    log_pdf = (
+        k_c * pt.log(r_c_safe)
+        + (k_c - 1.0) * pt.log(rt_safe)
+        - r_c * rt


In the log_pdf computation, the term "- r_c * rt" should use "rt_safe" instead of "rt" for consistency with the rest of the computation. When rt is negative (rt <= 0), rt_safe is set to epsilon, but the original rt (which is negative) is still used in this calculation. This could lead to incorrect log-likelihood values for edge cases where rt is barely positive after subtracting t.

Suggested change

- r_c * rt

- r_c * rt_safe

src/hssm/likelihoods/analytical.py

Copilot · 2026-01-28T14:05:28Z

src/hssm/likelihoods/analytical.py

+
+# set bounds
+poisson_race_params = ["r1", "r2", "k1", "k2", "t"]
+poisson_race_bounds = {param: (0.0, np.inf) for param in poisson_race_params}


The bounds for all parameters are set to (0.0, np.inf), which means the lower bound is 0.0 (inclusive). However, the parameter validation checks r1 > 0, r2 > 0, k1 > 0, and k2 > 0 (strictly greater than), meaning 0 is not actually a valid value. The bounds should be defined with an exclusive lower bound, or the implementation should use epsilon as the lower bound to be consistent with the parameter checks.

Suggested change

poisson_race_bounds = {param: (0.0, np.inf) for param in poisson_race_params}

poisson_race_bounds = {

"r1": (np.finfo(float).eps, np.inf),

"r2": (np.finfo(float).eps, np.inf),

"k1": (np.finfo(float).eps, np.inf),

"k2": (np.finfo(float).eps, np.inf),

"t": (0.0, np.inf),

}

Copilot · 2026-01-28T14:05:29Z

tests/test_likelihoods_poisson_race.py

+@pytest.mark.parametrize(
+    "param,bad_value",
+    [
+        ("r1", 0.0),
+        ("r2", -0.1),
+        ("k1", 0.0),
+        ("k2", -1.0),
+        ("t", -0.2),
+    ],
+)
+def test_poisson_race_parameter_validation(poisson_race_data, param, bad_value):
+    """Invalid parameter values should produce a ParameterValueError."""
+    assert_parameter_value_error(
+        poisson_race_data, theta_poisson_race, **{param: bad_value}
+    )


The test only validates that 0.0 and negative values are rejected, but doesn't test boundary values like very small positive numbers (e.g., 1e-10). Additionally, there's no test case for when t equals 0, which is a valid edge case according to the bounds (t >= 0) but might behave differently than other values.

Copilot · 2026-01-28T14:05:29Z

src/hssm/likelihoods/__init__.py

    "logp_full_ddm",
+    "logp_poisson_race",


The name 'logp_poisson_race' is exported by all but is not defined.

Suggested change

"logp_full_ddm",

"logp_poisson_race",

"logp_full_ddm",

AlexanderFengler · 2026-02-03T09:31:07Z

Overall looks like this is basically in shape, apart from some of the CoPilot comments (some of which seem worth addressing). thanks @hayden-johnson

fix doctring typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

initial poisson race implementation

5288e8f

AlexanderFengler reviewed Nov 2, 2025

View reviewed changes

src/hssm/likelihoods/analytical.py Outdated Show resolved Hide resolved

cpaniaguam reviewed Nov 11, 2025

View reviewed changes

src/hssm/likelihoods/analytical.py Outdated Show resolved Hide resolved

hayden-johnson added 3 commits November 16, 2025 17:54

add to public api

132e2e1

add basic test suite for poisson race

6c0e39f

fix formatting

5f91971

AlexanderFengler requested a review from cpaniaguam November 17, 2025 22:32

AlexanderFengler marked this pull request as ready for review November 17, 2025 22:32

cpaniaguam requested a review from Copilot November 18, 2025 21:04

Copilot AI reviewed Nov 18, 2025

View reviewed changes

src/hssm/likelihoods/analytical.py Outdated Show resolved Hide resolved

cpaniaguam requested changes Nov 18, 2025

View reviewed changes

tests/test_likelihoods_poisson_race.py Outdated Show resolved Hide resolved

hayden-johnson mentioned this pull request Dec 16, 2025

poisson race simulator lnccbrown/ssm-simulators#246

Open

minimal tutorial notebook

7a24aa1

hayden-johnson and others added 3 commits December 29, 2025 19:55

Update src/hssm/likelihoods/analytical.py

c62d4d8

fix shape bounds Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update tests/test_likelihoods_poisson_race.py

f2fc290

add pythonic syntax Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

fix formatting for test_likelihoods_poisson_race

776de18

hayden-johnson requested review from AlexanderFengler and cpaniaguam January 6, 2026 17:54

cpaniaguam requested changes Jan 8, 2026

View reviewed changes

cpaniaguam reviewed Jan 8, 2026

View reviewed changes

docs/tutorials/poisson_race.ipynb

Copy link

Collaborator

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notebooks looks good to me!

hayden-johnson and others added 4 commits January 22, 2026 11:17

build bound dicts with comprehension

f390d2c

Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>

fix logp_poisson_race docstring

737b069

fix docstring, add consitent safe usage

aeb123e

fix formatting

425aebc

hayden-johnson requested review from Copilot and cpaniaguam January 28, 2026 14:00

Copilot started reviewing on behalf of hayden-johnson January 28, 2026 14:00 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Update src/hssm/likelihoods/analytical.py

b5ef4bc

fix doctring typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

		pytensor.tensor.TensorVariable
		Per-trial log-likelihoods compatible with PyTensor graphs.

-poisson_race_bounds = {param: (0.0, np.inf) for param in poisson_race_params}
+poisson_race_bounds = {
+    "r1": (np.finfo(float).eps, np.inf),
+    "r2": (np.finfo(float).eps, np.inf),
+    "k1": (np.finfo(float).eps, np.inf),
+    "k2": (np.finfo(float).eps, np.inf),
+    "t": (0.0, np.inf),
+}

Conversation

hayden-johnson commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue #801

Uh oh!

codecov bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AlexanderFengler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

cpaniaguam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

review-notebook-app bot commented Dec 16, 2025

Uh oh!

hayden-johnson commented Dec 16, 2025

Uh oh!

cpaniaguam left a comment

Choose a reason for hiding this comment

Uh oh!

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

hayden-johnson Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

hayden-johnson Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

hayden-johnson Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cpaniaguam Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

hayden-johnson commented Oct 28, 2025 •

edited

Loading

codecov bot commented Oct 28, 2025 •

edited

Loading

hayden-johnson Jan 22, 2026 •

edited

Loading