Add dataloader pre-trained embeddings support to Merlin Models #1083
+963
−78
Conversation
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
Documentation preview |
gabrielspmoreira
force-pushed
the
tf/pretrained_emb
branch
from
1f1857c
to
1d13cb5
Compare
33 tasks
gabrielspmoreira
force-pushed
the
tf/pretrained_emb
branch
from
50a3ddd
to
78264e4
Compare
…trained embeddings
…96 to 100, in order to speed-up tests
…ence_testing_data
…d item is equal the item id cardinality
gabrielspmoreira
force-pushed
the
tf/pretrained_emb
branch
from
bd63bf3
to
c535e83
Compare
sararb
approved these changes
May 15, 2023
There was a problem hiding this comment.
Thank you for the PR @gabrielspmoreira! This looks good to me, I've just left some minor comments.
| @@ -96,7 +96,12 @@ def MLPBlock( | |||
|
|
|||
| for idx, dim in enumerate(dimensions): | |||
| dropout_layer = None | |||
| activation_idx = activation if isinstance(activation, str) else activation[idx] | |||
| activation = activation or "linear" | |||
There was a problem hiding this comment.
I understand that if activation is None, we will use "linear" by default. If yes, can we set the default value of activation in the method's args to None ?
merlin/models/tf/inputs/embedding.py
Outdated
| aggregation: Optional[TabularAggregationType], optional | ||
| Transformation block to apply for aggregating the inputs, by default None | ||
| block_name: str, optional | ||
| Name of the block, by default "embeddings" |
There was a problem hiding this comment.
Suggested change
| Name of the block, by default "embeddings" | |
| Name of the block, by default "pretrained_embeddings" |
merlin/models/tf/inputs/embedding.py
Outdated
| @@ -1238,6 +1335,8 @@ def process_str_sequence_combiner( | |||
| combiner = tf.keras.layers.Lambda(lambda x: tf.reduce_mean(x, axis=1)) | |||
| elif combiner == "sum": | |||
| combiner = tf.keras.layers.Lambda(lambda x: tf.reduce_sum(x, axis=1)) | |||
| elif combiner == "max": | |||
| combiner = tf.keras.layers.Lambda(lambda x: tf.reduce_max(x, axis=1)) | |||
| else: | |||
| raise ValueError( | |||
| "Only 'mean' and 'sum' str combiners is implemented for dense" | |||
There was a problem hiding this comment.
Suggested change
| "Only 'mean' and 'sum' str combiners is implemented for dense" | |
| "Only 'mean', 'sum', and 'max' str combiners is implemented for dense" |
| else: | ||
| inputs = tf.linalg.l2_normalize(inputs, axis=axis) | ||
| inputs = self._l2_norm(inputs) |
There was a problem hiding this comment.
we might need to specify the axis parameter in this line
| ) -> Union[tf.Tensor, tf.SparseTensor, tf.RaggedTensor]: | ||
| """Computes L2-norm for a given axis, typically axis = -1. | ||
| Equivalent to tf.linalg.l2_normalize(), but that function | ||
| does not support tf.RaggedTensor |
There was a problem hiding this comment.
Good to know, thanks for the fix!
marcromeyn
reviewed
May 16, 2023
| for col in schema: | ||
| table_name = col.name | ||
|
|
||
| tables[table_name] = NoOp() |
There was a problem hiding this comment.
Should this be configurable to allow users to do things like normalization?
marcromeyn
approved these changes
May 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1070, Fixes #1071, Fixes #1068, Fixes #1072, Fixes #1073
Goals ⚽
EmbeddingOperatortransform from Merlin dataloader: [RMP] Support pre-trained vector embeddings as input features into a model via the dataloader Merlin#211Tasks
BroadcastFeaturessupport expanding 2D pre-trained embeddings (contextual features) to concat with 3D sequence features #1073Implementation Details 🚧
PretrainedEmbeddingsblock that takes pre-trained embedding features and optionally projects them to a dim with a linear layer, applies a sequence aggregator and normalizes (e.g. with l2-norm). All of these options are configurableInputBlockV2was changed to accept an optionalpretrained_embeddingsargument, which by default selects features tagged with the EMBEDDING tag,PrepareListFeatureswas changed to define the shape of the last dim the pre-trained embeddings provided by the Loader, as that dim is None in graph mode.Testing Details 🔍