Skip to content

Change key type from prng_key to Array#2178

Merged
fehiepsi merged 4 commits intopyro-ppl:masterfrom
fehiepsi:typing
May 2, 2026
Merged

Change key type from prng_key to Array#2178
fehiepsi merged 4 commits intopyro-ppl:masterfrom
fehiepsi:typing

Conversation

@fehiepsi
Copy link
Copy Markdown
Member

@fehiepsi fehiepsi commented May 2, 2026

A key has type Array while key.dtype is a subclass of jax.dtypes.prng_key.

Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Collaborator

@juanitorduz juanitorduz left a comment

Choose a reason for hiding this comment

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

LGTM :)

(can you please ensure the lint is happy :) )

Copy link
Copy Markdown
Collaborator

@Qazalbash Qazalbash left a comment

Choose a reason for hiding this comment

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

LGTM!

Can we use an alias like PRNGKeyArray to distinguish between regular arrays and keys?

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@fehiepsi
Copy link
Copy Markdown
Member Author

fehiepsi commented May 2, 2026

I'm not sure if PRNGKey is an alias though. It seems jax.random.key returns an Array: https://github.com/jax-ml/jax/blob/2df8eced12c0e5a2456ebc0c693077109fa51fbd/jax/_src/random.py#L212-L214

@Qazalbash
Copy link
Copy Markdown
Collaborator

I'm not sure if PRNGKey is an alias though. It seems jax.random.key returns an Array: jax-ml/jax@2df8ece/jax/_src/random.py#L212-L214

I am not saying the alisa exists. My point was to use,

PRNGKeyArray: TypeAlias = Array

and use PRNGKeyArray when we are referring to Arrays that are keys.

@fehiepsi
Copy link
Copy Markdown
Member Author

fehiepsi commented May 2, 2026

Gotcha. I'm fine with either, slightly prefer using Array to be aligned with jax and shorter - the variable name already convey the "key" message.

@fehiepsi fehiepsi merged commit cd7c515 into pyro-ppl:master May 2, 2026
9 checks passed
@Qazalbash
Copy link
Copy Markdown
Collaborator

Let's see this in the future :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants