-
Notifications
You must be signed in to change notification settings - Fork 251
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
Adds Kokoro Build script for Keras-NLP GPU tests #1355
Conversation
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.
Looks good! Just two comments.
cd "src/github/keras" | ||
pip install -U pip setuptools | ||
|
||
if [ "$KERAS_BACKEND" == "tensorflow" ] |
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.
When I was working on updates for our old cloud build (which I will now close), I added this.
https://github.com/keras-team/keras-nlp/pull/1325/files#diff-656afe8203d34a7914ea21b11d467eddd6295aabf2ca2cdee2124c63984e680f
What to you think about adding that instead? It is kind of nice to be able to run pytest blah --check_gpu
locally as well (to sanity check that you are running with an accelerator).
If we did that could keep our bash scripts simpler, which is always a plus.
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.
Makes sense. Added.
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.
thanks!
kokoro/README.md
Outdated
@@ -0,0 +1 @@ | |||
CI to run on PR and merge to Master. |
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.
Not important, but up to now we have kept all our build configuration as private files .github
, .vscode
, .cloudbuild
. Is it possible to do the same here with .kokoro
? Or would that break tooling?
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.
That makes sense. It will not break tooling, but I need to update config and get approval from Kokoro. I can do that if we prefer to do that or we can do that in a subsequent change once we confirm everything is working.
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.
Renamed to .kokoro
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.
awesome thank you!
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.
Looks great! Glad we can share the requirements files in common with github actions.
Adds Kokoro Config and build script for Continuous (nightly) and Presubmit (PR) builds.