lnccbrown / lnccbrown/ssm-simulators
Custom boundary without `boundary_params` fails at `simulate()` with `KeyError: "Boundary 'my_boundary' is not registered"`
- Dominant language
- Jupyter Notebook
- Stars
- 24
- Forks
- 18
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 9
Description
`Simulator("ddm", boundary=my_callable)` with no `boundary_params` constructs without warning, because `ddm` ships `"boundary_params": []` and the guard checks key presence rather than emptiness. `make_boundary_dict` then tests `boundary_params` for truthiness, takes the registry branch, and looks the callable's `__name__` up in the boundary registry. The user is told their function is not registered and shown a list of six built-in boundary names, which points at the wrong fix — the actual fix is the one-line `boundary_params=[...]`. `ModelConfigBuilder.add_boundary` already rejects the same mistake at the point it is made.
Environment: ssm-simulators 0.13.2, Python 3.12, macOS (Darwin 25.4.0).
## Reproducer
```python
import numpy as np
from ssms.basic_simulators.simulator_class import Simulator
def my_boundary(t, theta=0.2, scale=1.0):
return scale * np.maximum(1.0 - theta * t, 0.1)
# "ddm" ships "boundary_params": [], so the missing-params warning never fires.
sim = Simulator("ddm", boundary=my_boundary)
print("boundary_name :", sim.config["boundary_name"])
print("boundary_params:", sim.config["boundary_params"])
sim.simulate(theta={"v": 0.5, "a": 1.0, "z": 0.5, "t": 0.3}, n_samples=10)
```
## Output
```
boundary_name : my_boundary
boundary_params: []
Traceback (most recent call last):
File "/…/snippet2.py", line 15, in
sim.simulate(theta={"v": 0.5, "a": 1.0, "z": 0.5, "t": 0.3}, n_samples=10)
File "/…/site-packages/ssms/basic_simulators/simulator_class.py", line 651, in simulate
boundary_dict = make_boundary_dict(model_config_local, theta)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/…/site-packages/ssms/basic_simulators/simulator.py", line 328, in make_boundary_dict
boundary_info = boundary_registry.get(boundary_name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/…/site-packages/ssms/config/boundary_registry.py", line 117, in get
raise KeyError(
KeyError: "Boundary 'my_boundary' is not registered. Available boundaries: ['addm_collapse', 'angle', 'conflict_gamma', 'constant', 'generalized_logistic', 'weibull_cdf']"
```
Related cases, from a second run that catches instead of crashing (verbatim excerpts):
```
shipped config for ddm boundary_params=[]
shipped config for addm boundary_params=[]
shipped config for angle boundary_params=''
=== B: Simulator('angle', boundary=), no boundary_params ===
warnings at construction: ["Custom boundary function provided without 'boundary_params'. You may need to specify bound"]
simulate RAISED KeyError: "Boundary 'my_boundary' is not registered. …"
=== C: documented form, boundary_params=['theta','scale'] ===
warnings at construction: []
OK -> rts[:3]: [0.548798 1.370101 1.313849]
=== D: parameter-free boundary with explicit boundary_params=[] ===
config['boundary_params']: []
simulate RAISED KeyError: "Boundary 'flat_boundary' is not registered. …"
=== E: ModelConfigBuilder.add_boundary(cfg, ) with no params ===
RAISED ValueError: Must provide boundary_params when using custom boundary function
```
Case D is the sharper edge: there is currently no value of `boundary_params` that expresses "my boundary takes no extra parameters" — `[]` fails identically to omitting it — so that shape of custom boundary is unreachable through `Simulator`.
## Why
- `ssms/basic_simulators/simulator_class.py:388-400` — callable branch of `_apply_custom_boundary`: sets `config["boundary_name"] = fn.__name__` (392), and warns + sets `config["boundary_params"] = []` only `if "boundary_params" not in config` (394-400). `ddm` and `addm` ship the key with value `[]`, so no warning.
- `ssms/basic_simulators/simulator_class.py:206-216` — `_build_config` applies `config.update(config_overrides)` before `_apply_custom_boundary`, so a user-supplied `boundary_params` and a base model's shipped one are indistinguishable at the point of the check.
- `ssms/basic_simulators/simulator.py:303` — `if callable(config.get("boundary")) and config.get("boundary_params"):` — truthiness, so `[]` falls through to the registry branch.
- `ssms/basic_simulators/simulator.py:326-328` → `ssms/config/boundary_registry.py:115-120` — registry lookup on the function's `__name__`, raising the misleading `KeyError`.
- Correct behaviour already exists at `ssms/config/model_config_builder.py:405-412`.
## Suggested fix
Two independent changes. First, dispatch on presence rather than truthiness so a callable boundary never falls through to a registry lookup on a function name, which also makes `boundary_params=[]` mean "no extra parameters":
```diff
- if callable(config.get("boundary")) and config.get("boundary_params"):
+ if callable(config.get("boundary")) and config.get("boundary_params") is not None:
```
Second, fail fast at construction the way `add_boundary` does, checking the user's own `config_overrides` rather than the merged config so a base model's shipped `[]` cannot suppress it:
```python
elif callable(boundary):
self._validate_boundary_function(boundary)
if "boundary_params" not in config_overrides:
raise ValueError(
f"Custom boundary {getattr(boundary, '__name__', 'custom')!r} requires an "
"explicit boundary_params list, e.g. Simulator('ddm', boundary=my_boundary, "
"boundary_params=['theta', 'scale']). Pass [] if it takes no extra parameters."
)
config["boundary_params"] = config_overrides["boundary_params"]
```
Optionally, in the else branch at `simulator.py:326-328`, if `config["boundary"]` is callable but the registry lookup fails, re-raise naming the real problem instead of the registry's "not registered" text.
## Is this intended?
The documented form works cleanly — case C above constructs with no warnings and simulates — so this is not a docs bug, and `config["boundary_name"] = fn.__name__` is defensible on its own as a readable metadata label. There is also already a warning on models whose config omits `boundary_params` (case B), which a maintainer may consider sufficient. The counter is that `ddm` and `addm` get no warning at all, and that even when the warning does fire it is emitted at construction while the failure arrives later at `simulate()`, misattributed to the registry.
Two caveats. The truthiness guard at `simulator.py:303` looks deliberate — the comment at 304-307 says it exists so process-local custom boundaries survive multiprocessing workers that start with a fresh registry; `is not None` preserves that intent, whereas dropping the second clause entirely would not. And the construction-time raise is a behaviour change for code that builds a `Simulator` with a callable boundary and only introspects `.config` without simulating; that combination is already non-functional, but it is technically a break. I exercised only the `Simulator` path — I did not survey whether `ssms.dataset_generators` or `hssm_support` reach `make_boundary_dict` with a callable boundary and empty `boundary_params`.
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with _apply_custom_boundary and _build_config in ssms/basic_simulators/simulator_class.py, then trace make_boundary_dict in ssms/basic_simulators/simulator.py and the validation in ssms/config/model_config_builder.py. Run the provided Simulator reproducer for omitted, empty, and explicit boundary_params; done means custom boundaries fail at construction when parameters are unspecified and valid parameter-free boundaries do not fall into registry lookup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100