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

Remove lmspmd2b and lmtransformeradam nightly runs #1025

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

RissyRan
Copy link
Collaborator

@RissyRan RissyRan commented Dec 8, 2023

Description

Remove lmspmd2b and lmtransformeradam nightly runs, as those tests have been running on Airflow. Failed with the same reason.

Tests

Please describe the tests that you ran on TPUs to verify changes.

Instruction and/or command lines to reproduce your tests: ...

Run ./scripts/gen-tests.sh

List links for your tests (use go/shortn-gen for any internal link): ...

Check schedule of the test has been updated, one example here.

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run one-shot tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

Copy link
Collaborator

@gkroiz gkroiz left a comment

Choose a reason for hiding this comment

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

Clever detail to assign scheduling date to Feb 31st haha. Is there a reason we don't outright delete these tests since they are now running on Airflow?

@RissyRan
Copy link
Collaborator Author

RissyRan commented Dec 8, 2023

Clever detail to assign scheduling date to Feb 31st haha. Is there a reason we don't outright delete these tests since they are now running on Airflow?

Thanks! Good question. Not like a few tests that are directly imported form Jsonnet in this repo to Airflow (i.e. JAX API and PT tests), Pax tests are safe to delete as we implement it from scratch. So removal works too! I guess the main reason I disable them instead of deletion is to keep them as example for you to onboard the rest 2 tests. I don't have preference. I am happy to remove it in this PR if you prefer.

@gkroiz
Copy link
Collaborator

gkroiz commented Dec 8, 2023

Clever detail to assign scheduling date to Feb 31st haha. Is there a reason we don't outright delete these tests since they are now running on Airflow?

Thanks! Good question. Not like a few tests that are directly imported form Jsonnet in this repo to Airflow (i.e. JAX API and PT tests), Pax tests are safe to delete as we implement it from scratch. So removal works too! I guess the main reason I disable them instead of deletion is to keep them as example for you to onboard the rest 2 tests. I don't have preference. I am happy to remove it in this PR if you prefer.

Let's remove, I can reference older commits of this repository when onboarding the other Pax models.

@RissyRan
Copy link
Collaborator Author

RissyRan commented Dec 9, 2023

Clever detail to assign scheduling date to Feb 31st haha. Is there a reason we don't outright delete these tests since they are now running on Airflow?

Thanks! Good question. Not like a few tests that are directly imported form Jsonnet in this repo to Airflow (i.e. JAX API and PT tests), Pax tests are safe to delete as we implement it from scratch. So removal works too! I guess the main reason I disable them instead of deletion is to keep them as example for you to onboard the rest 2 tests. I don't have preference. I am happy to remove it in this PR if you prefer.

Let's remove, I can reference older commits of this repository when onboarding the other Pax models.

Sounds good. Will do in a min.

@RissyRan RissyRan changed the title Disable lmspmd2b and lmtransformeradam nightly runs Remove lmspmd2b and lmtransformeradam nightly runs Dec 9, 2023
Copy link
Collaborator

@gkroiz gkroiz left a comment

Choose a reason for hiding this comment

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

LGTM

@RissyRan RissyRan merged commit a635d67 into GoogleCloudPlatform:master Dec 11, 2023
7 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.

2 participants