-
Notifications
You must be signed in to change notification settings - Fork 19.6k
Running TPU tests on linux-x86-ct6e-44-1tpu #21425
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
base: master
Are you sure you want to change the base?
Conversation
… hosted TPU based runner
updated build file path
updated build file path
Updated tpu_build job of actions.yml with specific runner label
Added container section
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #21425 +/- ##
==========================================
- Coverage 82.72% 76.92% -5.81%
==========================================
Files 567 572 +5
Lines 56245 57337 +1092
Branches 8790 8970 +180
==========================================
- Hits 46527 44104 -2423
- Misses 7561 11001 +3440
- Partials 2157 2232 +75
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Progress bar would always report the starting batch + 1 at the end of the batch. Now it takes into account `steps_per_execution` for the last batch reported. Fixes keras-team#20861
Using `keras.ops.math.logsumexp` with an int for `axis` in a functional model would throw an error.
…eras-team#21429) Arbitrary functions and classes are not allowed. - Made `Operation` extend `KerasSaveable`, this required moving imports to avoid circular imports - `Layer` no longer need to extend `KerasSaveable` directly - Made feature space `Cross` and `Feature` extend `KerasSaveable` - Also dissallow public function `enable_unsafe_deserialization`
…developed dtypes_new_test.py to use requires_tpu marker
…workflow and added a step to install docker client
…try after attaching service accounts as IAM policies
…ific backend deps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the same approach as OpenVino, i.e. have a file containing the list of failing tests:
- https://github.com/keras-team/keras/blob/master/conftest.py#L22-L51
- https://github.com/keras-team/keras/blob/master/keras/src/backend/openvino/excluded_concrete_tests.txt
- https://github.com/keras-team/keras/blob/master/.github/workflows/actions.yml#L108-L112
So for now you'd exclude anything that doesn't pass. The file would be it will be at the root, something like tpu_excluded_tests.txt.
| pip install -r requirements.txt --progress-bar off --upgrade | ||
| if [ "${{ matrix.nnx_enabled }}" == "true" ]; then | ||
| pip install --upgrade git+https://github.com/google/flax.git | ||
| pip install --upgrade flax>=0.11.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be reverted
| @@ -0,0 +1,104 @@ | |||
| # name: TPU Tests | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove all commented out code
| runs-on: linux-x86-ct6e-44-1tpu | ||
|
|
||
| container: | ||
| # The container image is now set to python:3.10-slim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this comment
| - name: Install System Dependencies | ||
| run: | | ||
| apt-get update && apt-get install -y --no-install-recommends \ | ||
| git \ | ||
| sudo \ | ||
| && rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed. I got it to work without this:
https://github.com/keras-team/keras-rs/blob/main/.github/workflows/actions.yml#L49
| - name: Install Dependencies | ||
| run: | | ||
| pip install --no-cache-dir -U pip setuptools && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove setuptools, we're not building the wheel.
| ENV KERAS_HOME=/github/workspace/.github/workflows/config/jax \ | ||
| KERAS_BACKEND=jax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be here since we want to support TF too.
| RUN pip install --no-cache-dir -U pip setuptools && \ | ||
| pip install --no-cache-dir -U psutil && \ | ||
| pip install --no-cache-dir -r requirements-jax-tpu.txt && \ | ||
| pip uninstall -y keras keras-nightly | ||
| # python3 -c 'import jax;print(jax.__version__);print(jax.default_backend())' && \ | ||
| # python3 -c 'import jax;assert jax.default_backend().lower() == "tpu"' && \ | ||
| # pytest keras --ignore keras/src/applications \ | ||
| # --ignore keras/src/layers/merging/merging_test.py \ | ||
| # --cov=keras \ | ||
| # --cov-config=pyproject.toml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, this will be done by the action.
| @@ -0,0 +1,31 @@ | |||
| pre-commit | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't you use the requirements-common.txt?
| # for tree_test.py | ||
| dm_tree | ||
| coverage | ||
| coverage!=7.6.5 # 7.6.5 breaks CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this file.
| #tensorflow==2.18.0 | ||
| #--find-links https://storage.googleapis.com/libtpu-tf-releases/index.html | ||
| #tensorflow-tpu==2.18.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use tensorflow-tpu==2.19.1.
Running TPU tests on linux-x86-ct6e-44-1tpu