Skip to content

Restore the global RKG key when a transform raises - #96

Merged
cemde merged 1 commit into
liukidar:mainfrom
cemde:fix/71-rkg-tracer-leak
Aug 11, 2026
Merged

Restore the global RKG key when a transform raises#96
cemde merged 1 commit into
liukidar:mainfrom
cemde:fix/71-rkg-tracer-leak

Conversation

@cemde

@cemde cemde commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Bug

Inside a transform, _wrap_fn swaps the global RKG.key for the traced key so that layers constructed within the transform draw from the traced stream, then swaps it back:

_old_key, RKG.key = RKG.key, kwargs["__RKG"].key
_r = fn(*args, **kwargs, _is_root=False)
RKG.key = _old_key          # plain statement, not a finally

If fn raises, the restore never runs and the tracer stays in module-level state.

Impact

pcx.RKG is the default argument of every layer and Vode constructor, so a leaked tracer breaks every subsequent random draw in the process with UnexpectedTracerError, including in code that never touched a transform. One failed pxf.jit call is enough. Under pytest, one failing test breaks every later test that constructs a layer.

Fix

try/finally around the call, at both swap sites:

try:
    _r = fn(*args, **kwargs, _is_root=False)
finally:
    RKG.key = _old_key

The success path is unchanged; return _r, kwargs stays outside the try. The flow transforms (scan, while_loop, cond, switch) inherit _BaseTransform and route through the same closures, so both guarding tests are covered by these two edits.

Nesting restores correctly in LIFO order: for jit(value_and_grad(f)) the inner finally restores the outer transform's tracer, and the outer finally restores the concrete key.

Follow-up, not fixed here

Vmap._t has the same shape of bug at a different site: it splits the global key over the batch axis and merges it back with the merge outside any try, so a raising pxf.vmap leaves the key with an extra leading axis. Concrete rather than traced, so this PR's guarding test (which asserts "not a tracer") passes while the process is still poisoned. Different function, different fix, filed as #91.

Closes #71

_BaseTransform swapped RKG.key for the traced key and restored it with a
plain statement, so a transformed function that raised left the tracer
installed in module-level state. Since pcx.RKG is the default argument of
every layer and Vode constructor, every later random draw in the process
then failed with UnexpectedTracerError.
@cemde
cemde force-pushed the fix/71-rkg-tracer-leak branch from 33fdbd0 to 1f38d99 Compare August 9, 2026 18:23
@cemde
cemde requested a review from liukidar August 9, 2026 18:34

@liukidar liukidar left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lgtm

@cemde
cemde merged commit a742861 into liukidar:main Aug 11, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An exception inside a transform poisons the global RNG for the rest of the process

2 participants