Skip to content

Commit 00e8f81

Browse files
committed
Fix Slice sampler bugs introduced in b988ba9
1. The previous interval was of the form `[q0 - uniform(0, self.w), q0 + self.w]` which is not symmetric around q0 in expectation, breaking detailed balance. 2. The left/right bound values used for updating earlier variables were only set to the accepted values during tuning. This might have made bug 1 worse.
1 parent de55543 commit 00e8f81

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

RELEASE-NOTES.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,16 +148,16 @@ This includes API changes we did not warn about since at least `3.11.0` (2021-01
148148
- Added the low level `compile_forward_sampling_function` method to compile the aesara function responsible for generating forward samples (see [#5759](https://github.com/pymc-devs/pymc/pull/5759)).
149149
- ...
150150

151-
152-
## Documentation
151+
### Documentation
153152
- Switched to the [pydata-sphinx-theme](https://pydata-sphinx-theme.readthedocs.io/en/latest/)
154153
- Updated our documentation tooling to use [MyST](https://myst-parser.readthedocs.io/en/latest/), [MyST-NB](https://myst-nb.readthedocs.io/en/latest/), sphinx-design, notfound.extension,
155154
sphinx-copybutton and sphinx-remove-toctrees.
156155
- Separated the builds of the example notebooks and of the versioned docs.
157156
- Restructured the documentation to facilitate learning paths
158157
- Updated API docs to document objects at the path users should use to import them
159158

160-
### Internal changes
159+
### Maintenance
160+
- ⚠ Fixed old-time bug in Slice sampler that resulted in biased samples (see [#5816](https://github.com/pymc-devs/pymc/pull/5816)).
161161
- ⚠ PyMC now requires Scipy version `>= 1.4.1` (see [4857](https://github.com/pymc-devs/pymc/pull/4857)).
162162
- Removed float128 dtype support (see [#4514](https://github.com/pymc-devs/pymc/pull/4514)).
163163
- Logp method of `Uniform` and `DiscreteUniform` no longer depends on `pymc.distributions.dist_math.bound` for proper evaluation (see [#4541](https://github.com/pymc-devs/pymc/pull/4541)).

pymc/step_methods/slicer.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,11 @@ def astep(self, q0, logp):
7373
# uniformly sample from 0 to p(q), but in log space
7474
q_ra = RaveledVars(q, q0.point_map_info)
7575
y = logp(q_ra) - nr.standard_exponential()
76-
ql[i] = q[i] - nr.uniform(0, self.w[i])
77-
qr[i] = q[i] + self.w[i]
76+
77+
# Create initial interval
78+
ql[i] = q[i] - nr.uniform() * self.w[i] # q[i] + r * w
79+
qr[i] = ql[i] + self.w[i] # Equivalent to q[i] + (1-r) * w
80+
7881
# Stepping out procedure
7982
cnt = 0
8083
while y <= logp(
@@ -104,16 +107,16 @@ def astep(self, q0, logp):
104107
if cnt > self.iter_limit:
105108
raise RuntimeError(LOOP_ERR_MSG % self.iter_limit)
106109

107-
if (
108-
self.tune
109-
): # I was under impression from MacKays lectures that slice width can be tuned without
110+
if self.tune:
111+
# I was under impression from MacKays lectures that slice width can be tuned without
110112
# breaking markovianness. Can we do it regardless of self.tune?(@madanh)
111113
self.w[i] = self.w[i] * (self.n_tunes / (self.n_tunes + 1)) + (qr[i] - ql[i]) / (
112114
self.n_tunes + 1
113-
) # same as before
114-
# unobvious and important: return qr and ql to the same point
115-
qr[i] = q[i]
116-
ql[i] = q[i]
115+
)
116+
117+
# Set qr and ql to the accepted points (they matter for subsequent iterations)
118+
qr[i] = ql[i] = q[i]
119+
117120
if self.tune:
118121
self.n_tunes += 1
119122
return q

0 commit comments

Comments
 (0)