Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update manifest and pin newer numpy in containers #444

Merged
merged 4 commits into from
Jan 4, 2024
Merged

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Jan 3, 2024

As of #405 the presubmit CI is based on package versions listed in the manifest.yaml that is committed to the repository. This has not been updated for ~1 month, so the presubmit CI is testing ~1 month old versions of the ecosystem. This PR updates it using the commit from https://github.com/NVIDIA/JAX-Toolbox/tree/znightly-2024-01-03-7395605285, generated by the nightly CI run.

Because this bumps the JAX version by ~1 month, we have to include fixes for deprecations. In particular replacing jax.random.KeyArray with plain jax.Array (nvjax-svc-0/t5x@4d5ec2f).

The deprecated name is used in older versions of the chex package, which are being selected by pip's dependency resolver despite newer versions being available. We avoid this by giving pip a helping hand and nudging it to use a newer numpy version, which allows it to select a newer chex. But it's easy to imagine similar issues in future with other packages.

Closes #448.

@olupton olupton marked this pull request as ready for review January 3, 2024 17:59
@olupton olupton requested review from terrykong and yhtang and removed request for terrykong January 3, 2024 17:59
@olupton olupton changed the title Pin newer numpy in containers Update manifest and pin newer numpy in containers Jan 4, 2024
Copy link
Contributor

@terrykong terrykong left a comment

Choose a reason for hiding this comment

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

The change LGTM. FFTM assuming presubmit doesn't show any regression

Copy link
Collaborator

@yhtang yhtang left a comment

Choose a reason for hiding this comment

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

LGTM

@olupton olupton merged commit 2c2d7f9 into main Jan 4, 2024
95 of 100 checks passed
@olupton olupton deleted the olupton/numpy branch January 4, 2024 21:03
@terrykong
Copy link
Contributor

This wasn't caught in the review, but just wanted to ask the next time the patch branches are updated, to mention that in the PR description. It wasn't clear that mirror/patch/t5x_te_in_contrib_noindent was swapped for mirror/ashors/fix_rng_dtype, where the former is the "official" TE branch.

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.

T5X rosetta nightlies are broken
5 participants