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

Simplify running KerasNLP with Keras 3 #1308

Merged
merged 5 commits into from
Nov 29, 2023

Conversation

mattdangerw
Copy link
Member

@mattdangerw mattdangerw commented Nov 9, 2023

We should not land this until Keras 3 and TensorFlow 2.15 are released.

This deprecates any public use of keras_core, keras_core becomes an internal shim solely to backports keras.ops to Keras 2. This updates our installation instructions for the Keras 3 world.

@mattdangerw
Copy link
Member Author

/gcbrun

@mattdangerw mattdangerw requested a review from fchollet November 9, 2023 19:55
_USE_KERAS_3 = detect_if_tensorflow_uses_keras_3()
if _USE_KERAS_3:
_MULTI_BACKEND = True
if not _USE_KERAS_3:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is totally optional. We don't need to have it, but since we used to switch to keras core if KERAS_BACKEND was set, this seems like a useful signpost.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good to have IMO!

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
_USE_KERAS_3 = detect_if_tensorflow_uses_keras_3()
if _USE_KERAS_3:
_MULTI_BACKEND = True
if not _USE_KERAS_3:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good to have IMO!

raise ValueError(
"Lora only works with multi-backend Keras 3. Please set the "
"`KERAS_BACKEND` environment variable to use this API."
"Lora requires with Keras 3, and Keras 2 is installed. Please "
Copy link
Collaborator

Choose a reason for hiding this comment

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

requires Keras 3, but

@mattdangerw mattdangerw force-pushed the simplify-keras-3-support branch from 3cce43b to 7e4c66e Compare November 21, 2023 23:47
@mattdangerw mattdangerw marked this pull request as ready for review November 28, 2023 22:53
We should not land this until Keras 3, TensorFlow 2.15, and
keras-nlp-nightly are released.
@mattdangerw mattdangerw force-pushed the simplify-keras-3-support branch from 7e4c66e to e214d43 Compare November 28, 2023 23:01
@mattdangerw
Copy link
Member Author

/gcbrun

@mattdangerw mattdangerw merged commit b6a6e27 into keras-team:master Nov 29, 2023
6 of 9 checks passed
mattdangerw added a commit to mattdangerw/keras-hub that referenced this pull request Dec 7, 2023
* Simplify running KerasNLP with Keras 3

We should not land this until Keras 3, TensorFlow 2.15, and
keras-nlp-nightly are released.

* Address comments

* Tweaks

* Add link

* fix link
mattdangerw added a commit to mattdangerw/keras-hub that referenced this pull request Dec 7, 2023
* Simplify running KerasNLP with Keras 3

We should not land this until Keras 3, TensorFlow 2.15, and
keras-nlp-nightly are released.

* Address comments

* Tweaks

* Add link

* fix link
mattdangerw added a commit to mattdangerw/keras-hub that referenced this pull request Dec 7, 2023
* Simplify running KerasNLP with Keras 3

We should not land this until Keras 3, TensorFlow 2.15, and
keras-nlp-nightly are released.

* Address comments

* Tweaks

* Add link

* fix link
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