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

tf.keras Serialization fix #432

Merged
merged 3 commits into from
May 24, 2022
Merged

tf.keras Serialization fix #432

merged 3 commits into from
May 24, 2022

Conversation

MasterSkepticista
Copy link
Collaborator

@MasterSkepticista MasterSkepticista commented May 18, 2022

This closes #422.

Serialization of tf.keras models is natively supported as of keras-team/keras#14748.
This PR removes hotfix function to allow serialization on TF2.7 and greater.

* Remove `layers.py`, declare model/opt/loss in notebook
* Update shape blobs to MNIST 2D input shape

Signed-off-by: Shah, Karan <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2022

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@mansishr
Copy link
Collaborator

Hi @MasterSkepticista, thanks for linking a PR. I had talked to @psfoley regarding the issue and having a tensorflow version check before invoking the hotfix function might be a better way to go about it. This way, we can support tutorials having older tensorflow versions as well.

@MasterSkepticista
Copy link
Collaborator Author

MasterSkepticista commented May 19, 2022

@mansishr Version check is a possible solution.
More generally, I don't see any tutorials failing with the change so long as TF is above or equal 2.7

If there are existing outside projects using lower TF versions, a version check will be required.

@mansishr
Copy link
Collaborator

@MasterSkepticista that's right. I'll submit a PR with a version check before invoking the hotfix function. Thanks for your help!

@psfoley
Copy link
Contributor

psfoley commented May 19, 2022

@MasterSkepticista A version check would be useful here for the reason that you mentioned; external projects may rely on an earlier TF version. @mansishr if you want to create a separate PR for that, then I think once cell outputs are cleared in the jupyter notebook this is ready to merge

Signed-off-by: Shah, Karan <[email protected]>
@psfoley
Copy link
Contributor

psfoley commented May 23, 2022

@MasterSkepticista could you please sign the CLA?

@MasterSkepticista
Copy link
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

@mansishr
Copy link
Collaborator

@psfoley @MasterSkepticista I've created a PR #435 for TF version check.

@psfoley psfoley merged commit dc63dcb into develop May 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2022
@MasterSkepticista MasterSkepticista deleted the tf_keras_pickle branch May 26, 2022 16:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BatchNormalization layer fails to deserialize when working with Tensorflow
3 participants