Should we split model into Model & SequentialModel?

[marcromeyn]

Should we split model into Model & SequentialModel?

Right now the Model class does 2 things:

1. Overwrite methods like fit, evaluate to allow for data passed in as merlin.io.Dataset.
2. Take in one or multiple blocks/layers to call sequentially.

This begs the question whether we should split up model in 2 different classes:

1. Model (just with the methods to overwrite)
2. SequentialModel which inherits from Model and calls it's inputs sequentially.

A big advantage of this split is that we would be able to handle users wanting to create their model using the sub-class API for more control. This is technically possible currently as well but a bit awkward. A user could create a two-tower model with something like:

class TwoTowerModel(Model):
   def __init__(self, schema):
       user_inputs = InputBlock(schema.select_by_tag(Tags.USER))
       self.user = user_inputs.connect(MLPBlock([512, 256])

       item_inputs = InputBlock(schema.select_by_tag(Tags.ITEM))
       self.item = item_inputs.connect(MLPBlock([512, 256])

       self.prediction = ItemRetrievalTask(schema)
   
    def call(self, schema):
       user = self.user(inputs)
       item = self.item(inputs)

       return self.prediction({"query": user, "item": item})

One implication of this change would be that our RetrievalModel class becomes a bit weird. Right now we are only using that to add some extra methods to export embeddings. One way to deal with this is to remove this class & to move these methods to standard functions that take the model in as first arg.

from merlin.models import tf as mm

model = mm.TwoTowerModel(schema, mm.MLPBlock([512, 256]))
model.fit(...)

# Current
model.user_embeddings(dataset)

# Potenial new way
mm.get_user_embeddings(model, dataset)