Make rkg(1) return the same shape as rkg(n) - #99
Merged
Conversation
cemde
force-pushed
the
fix/75-rkg-shape
branch
2 times, most recently
from
August 9, 2026 14:29
bd3ca10 to
3fce799
Compare
cemde
marked this pull request as draft
August 9, 2026 14:49
cemde
marked this pull request as ready for review
August 9, 2026 15:05
rkg(1) special-cased n == 1 and returned a bare key of shape (2,), so keys[0] was a uint32 and iterating yielded the key's two integers. Any batched path hit this on a final partial batch of size 1. n now defaults to None, which means "one unbatched key" as before, while an explicit n always yields shape (n, 2). Closes liukidar#75
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.
Bug
The convenience unwrap for a single key was keyed off
n == 1, which is also the default. So "give me one key" and "give me a batch of n keys" were conflated atn == 1.Impact
rkg(n)returns shape(n, 2)for n >= 2 but a bare key of shape(2,)for n == 1. Sokeys[0]is auint32rather than a key, and iterating yields the key's two integers rather than one key. Any batched path hits this on a final partial batch of size 1, or a single-example debug run, and has to special-case it with nothing in the API to suggest so.Fix
Make the default a
Nonesentinel, so the unwrap keys off "no argument given" rather than off the value 1:rkg()is unchanged.rkg(1)now returns(1, 2)like every othern.Why the signature had to change
The two contracts are only separable if the default stops being the literal
1.test_keys_are_usable_by_jax_randomdoesjax.random.normal(rkg(), (16,)), and jax rejects a(1, 2)key there, so the no-argument call must keep returning a bare key.Breaking change
rkg(1)previously returned a scalar-indexable bare key, and one shipped example relied on that.examples/7_mcpc.ipynbhadseed: int = lambda: px.RKG(1)[0], where[0]on a(2,)key produced the scalar seedoptax.add_noisewants. Under the new contract that expression yields shape(2,)and optax raisesTypeError: key accepts a scalar seed.Updated in this PR to
px.RKG()[0], which is the no-argument form and gives the scalar back. Any downstream code callingrkg(1)to mean "one bare key" needs the same edit.Both internal callers (
LinearandConv2d, viapcx/nn/_layer.py) use the no-argument form and are unaffected.Vmap._tmanipulatesRKG.key.split(...)directly and bypasses__call__.Closes #75