fix: Ensure eval mode for farm and transformer models for predictions

[sjrl]

Related Issues

• fixes N/A
• Related to PR fix: Ensure eval mode for TableReader model for predictions #3743

Proposed Changes:

I added model.eval() calls to make sure to set the models in:

• FARMReader
• TransformerReader
• EmbeddingRetriever (both with the _RetribertEmbeddingEncoder and _DefaultEmbeddingEncoder)
• PromptNode
• TransformersDocumentClassifier
• TransformersTranslator
• Text2SparqlRetriever
• Text2Speech
• EntityExtractor

are set to eval mode when running an inference prediction. Otherwise, currently, if the model is set to train mode the predictions become random due to layers like dropout, and BatchNormalization being present in the underlying model architecture. This is something we already do for some of our nodes like the DensePassageRetriever.

How did you test it?

I added unit tests that cover:

• FARMReader and TransformerReader
• EmbeddingRetriever
• Text2Speech

to make sure the nodes provide correct results even if they are set to train mode before running a prediction. I did confirm that these unit tests fail without the new changes.

Notes for the reviewer

• This probably was not noticed before because the model is set to eval mode by default when loading it. However, this error could have occurred when using the nodes right after training. For example, using the predict function of the FARMReader right after training.
• This was not an issue when using a SentenceTransformer model since the SentenceTransformer.encode function sets the underlying PyTorch model to eval mode within the function call.

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added tests that demonstrate the correct behavior of the change
• I've used the conventional commit convention for my PR title
• I documented my code
• I ran pre-commit hooks and fixed any issue

[vblagoje]

@sjrl looks good from the outset. Any other pipelines we use in the codebase? Let's not switch those explicitly to eval, as it's already done automatically.

[vblagoje]

@sjrl wait wait, I don't agree we call the eval() switch on every model invocation. That's excessive; why do that? Take the example of an encoder, we need to encode millions of documents, and yet we'll call eval on every call. I would respectfully disagree.

[sjrl]

wait wait, I don't agree we call the eval() switch on every model invocation. That's excessive; why do that? Take the example of an encoder, we need to encode millions of documents, and yet we'll call eval on every call. I would respectfully disagree.

Hmm, well considering that Sentence Transformers actually does this for their sentence encoder models already whenever you call the function encode

    def encode(self, sentences: Union[str, List[str]],
               batch_size: int = 32,
               show_progress_bar: bool = None,
               output_value: str = 'sentence_embedding',
               convert_to_numpy: bool = True,
               convert_to_tensor: bool = False,
               device: str = None,
               normalize_embeddings: bool = False) -> Union[List[Tensor], ndarray, Tensor]:
        """
        Computes sentence embeddings
        :param sentences: the sentences to embed
        :param batch_size: the batch size used for the computation
        :param show_progress_bar: Output a progress bar when encode sentences
        :param output_value:  Default sentence_embedding, to get sentence embeddings. Can be set to token_embeddings to get wordpiece token embeddings. Set to None, to get all output values
        :param convert_to_numpy: If true, the output is a list of numpy vectors. Else, it is a list of pytorch tensors.
        :param convert_to_tensor: If true, you get one large tensor as return. Overwrites any setting from convert_to_numpy
        :param device: Which torch.device to use for the computation
        :param normalize_embeddings: If set to true, returned vectors will have length 1. In that case, the faster dot-product (util.dot_score) instead of cosine similarity can be used.
        :return:
           By default, a list of tensors is returned. If convert_to_tensor, a stacked tensor is returned. If convert_to_numpy, a numpy matrix is returned.
        """
        self.eval()
        ...

which we call here in Haystack

haystack/haystack/nodes/retriever/_embedding_encoder.py

Line 207 in fa78e2b

emb = self.embedding_model.encode(

we are in fact already calling it every time at model invocation time for our EmbeddingRetriever node whenever we use Sentence Transformers (which we use most often in dC) and has not seemed to impact our timings.

I don't think looping through all layers in a model takes that long, but I can collect some timings on some of the Flan Models to double check.

[vblagoje]

Ok ok @sjrl , I wouldn't want you to waste your time on such tests now. Let's just get a nod from other team members as well. I am convinced by your proofs, but HF also doesn't switch the pipeline to eval on every call. Maybe it's an omission. Let's confirm this change with other team members. cc @mayankjobanputra @julian-risch @bogdankostic

[bogdankostic]

I don’t have much experience with this but I suppose that setting a model into eval mode shouldn’t be a heavy operation as it only affects a small fraction of layers like Dropout for example.
On the other hand, I’m not really sure if this is really needed for every node. I see that it’s useful for FARMReader because there we provide our own train method that sets the model to training mode. For the other nodes where we don’t provide our own train method, we allow users to load a model only using a model identifier or a local path and pass the task of actually loading the model to transformers - where it is set to eval model by default. The users must have therefore set the models to training mode explicitly themselves, and I think we can expect them to set it back to eval mode as well - but that's just my point of view, interested to hear what others think about this.

[vblagoje]

I echo Bogdan's comments. If we were a train-heavy framework - let's go for it. But we only train a few components! Are you concerned about some nefarious user actions @sjrl ? What motivates this change? Consistency? What else?

[julian-risch]

It looks to me as if self.train() and self.eval() just set a boolean for the module and all its children ( = neural network layers).
https://github.com/pytorch/pytorch/blob/d24324bf1d2b921c9c631022c9009f4840ee6acb/torch/nn/modules/module.py#L2265
The forward pass of the modules takes into account the value of the boolean, for example, here: https://github.com/pytorch/pytorch/blob/d24324bf1d2b921c9c631022c9009f4840ee6acb/torch/nn/modules/batchnorm.py#L161
So the self.eval() operation itself shouldn't take long. However, I also think that calling self.eval() when a model is loaded should be enough.
In Haystack, we call self.model.train() in the beginning of the training here and thereby set the model into training mode, right? Maybe we could call self.model.eval() at the end of the training before returning the trained model here? Basically ensuring training mode temporarily during training and ensuring eval mode everywhere else because models are loaded in eval mode as Bogdan pointed out.

[bogdankostic]

Maybe we could call self.model.eval() at the end of the training before returning the trained model here? Basically ensuring training mode temporarily during training and ensuring eval mode everywhere else because models are loaded in eval mode as Bogdan pointed out.

I like this idea!

[vblagoje]

@sjrl what do you think about the ideas ☝️ ?

[sjrl]

Maybe we could call self.model.eval() at the end of the training before returning the trained model here? Basically ensuring training mode temporarily during training and ensuring eval mode everywhere else because models are loaded in eval mode as Bogdan pointed out.

This sounds good to me! I'll go ahead and change the PR to do this instead. I'm fairly busy this week so I might not be able to get to it until next week.

[sjrl]

Hey @vblagoje sorry that this took me so long, but it is finished now! I've made the changes as discussed in this PR and it is ready for another review.

[vblagoje]

LGTM @sjrl , let's resolve this conflict and integrate this one

[coveralls]

Coverage: 38.92% (-0.002%) from 38.921% when pulling 2002b65 on inf-model-eval into 8228081 on main.