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

Update embedding_bag.py for save_model in 'tf' mode #2804

Conversation

AlexanderLavelle
Copy link
Contributor

self.input_length is not defined, which affects the pickling / unpickling of this layer in tf model mode.

Description

Brief Description of the PR:

Of the two options of adding input_length or removing it entirely, it seems more elegant to remove it. The attribute input_length was likely ported from TF Embedding Layer, where it might be necessary; however, in EmbeddingBag there does not appear to be such requirements.

The alternative way to solve the issue is to take input_length as an int argument set to None but this seems likely to be more confusing and would additionally require a modification to the documentation

Fixes # (issue)
2803

Type of change

Checklist:

  • [x ] I've properly formatted my code according to the guidelines
    • By running Black + Flake8
    • By running pre-commit hooks
  • [x ] This PR addresses an already submitted issue for TensorFlow Addons
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • This PR contains modifications to C++ custom-ops

How Has This Been Tested?

If you're adding a bugfix or new feature please describe the tests that you ran to verify your changes:

  • This is a simple modification of the removal of a line which is unused. If you look in VS Code you would see that the original is undefined. This layer now works as intended in save_model and load_model.

`self.input_length` is not defined, which affects the pickling / unpickling of this layer in `tf` model mode.
@boring-cyborg boring-cyborg bot added the layers label Jan 31, 2023
@bot-of-gabrieldemarmiesse

@Rocketknight1

You are owner of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@bhack bhack linked an issue Jan 31, 2023 that may be closed by this pull request
Copy link
Contributor

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, and sorry for leaving this issue in! The change seems good to me.

@fsx950223 fsx950223 merged commit 104e345 into tensorflow:master Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EmbeddingBag saving/loading in 'tf' save_model mode
4 participants