Skip to content

Conversation

@jakevdp
Copy link
Contributor

@jakevdp jakevdp commented Oct 6, 2023

The correct annotation for PRNG keys is jax.Array (see JEP 9263). Note that jax.random.KeyArray has always been effectively aliased to Any at type checking time, so changing these to jax.Array makes the annotations more specific than they were previously.

I updated the version pinning to jax>=0.4.1 (released December 2022) because this is the first release where jax.Array was available as a type annotation.

@hvaara
Copy link
Contributor

hvaara commented Oct 6, 2023

Thanks for proposing this upstream, @jakevdp! 😄

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks a lot!

Would you recommend to start going over uses of jax.random.PRNGKey() and replacing them with jax.random.key() now?

@DN6
Copy link
Collaborator

DN6 commented Oct 9, 2023

@pcuenca Good to merge this?

@pcuenca
Copy link
Member

pcuenca commented Oct 9, 2023

Yes! Maybe @patrickvonplaten or @yiyixuxu would like to take a quick look.

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Looks good to me! Happy to bump JAX here to 0.4!

Thanks a lot @jakevdp

@patrickvonplaten patrickvonplaten merged commit a844065 into huggingface:main Oct 9, 2023
@jakevdp jakevdp deleted the KeyArray branch October 9, 2023 15:34
@jakevdp
Copy link
Contributor Author

jakevdp commented Oct 9, 2023

Thanks!

yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
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.

5 participants