-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
adapting the script movielens_recommendations_transformers.py to be Backend-Agnostic #2039
Open
Humbulani1234
wants to merge
5
commits into
keras-team:master
Choose a base branch
from
Humbulani1234:adapt_movielens_recommendations_transformers
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
be4b055
adapting the script movielens_recommendations_transformers.py to be B…
Humbulani1234 2d7d856
adjusting the script to be more modular
Humbulani1234 cfdf699
introducing some efficiency
Humbulani1234 0e5fed2
addressing PR comments
Humbulani1234 4dbba56
removed the extensive refactoring
Humbulani1234 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Did the old really need extensive refactoring? Best I can tell only this line needed to change (to become
ops.arange()
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.
Changing only the
tf.range
toops.arange
does not work and I believe it is becauseStringLookup
is part of the model, not oftf.data.Dataset
in the original script, and only Tensorflow can handle strings.So, my approach was to split the function
encode_input_features
into two partsStringLookups
andEmbeddings
. I placed theStringLookups
functionality intotf.data.Dataset
processing step, and created a class to handle each inputembeddings
. And also the issue that this script must run whether one chooses to includeuser_features
or not also adds some refactoring.However, if there is a less-refactoring approach/method, I'm wiling to learn and implement it. I must confess also that the refactoring felt a bit extensive, but required by my approach.