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

NeuralModel loading method doesn't work for CNNs #89

Closed
hugo-quantmetry opened this issue Jun 28, 2021 · 1 comment
Closed

NeuralModel loading method doesn't work for CNNs #89

hugo-quantmetry opened this issue Jun 28, 2021 · 1 comment

Comments

@hugo-quantmetry
Copy link
Contributor

Python version :
3.6

Melusine version :
2.3.1

Operating System :
Mac

Steps to reproduce :
The code below does the following:

  • Train a model
  • Save the model
  • (Re)Load the model
  • Predict with both the original model and the reloaded model

The predictions should be equal but they are different

from melusine.models.neural_architectures import cnn_model
from melusine.models.train import NeuralModel
from melusine.nlp_tools.embedding import Embedding

X, y = MY_DATA
pretrained_embedding = Embedding().load("MY_EMBEDDING")

model = NeuralModel(
    architecture_function = cnn_model,
    pretrained_embedding = pretrained_embedding,
)
model.fit(X, y)
model.save_nn_model("MY_MODEL")

# Load the saved model
loaded_model = NeuralModel(
    architecture_function = cnn_model,
    pretrained_embedding = pretrained_embedding,
)
loaded_model.load_nn_model("MY_MODEL")

y_pred_base = model.predict(X)
y_pred_loaded = loaded_model.predict(X)

print((y_pred_base == y_pred_loaded).all())

Bug explanation :
When making a prediction (NeuralModel.predict), the NeuralModel uses the attribute vocabulary_dict.
This attribute is an empty dict at model initialization and it is filled when the model is fitted.

The vocabulary_dict attribute is not saved by the save_nn_model method,
therefore when the model is loaded and the predict method is called, all the tokens are mapped to the unknown token and the predicted values make no sens.

Suggestions :
Quick Fix : If you are in a rush, you can use the following code to save and load the model:

import joblib
model.save_nn_model("MY_MODEL")
joblib.dump(model, "MY_MODEL.pkl", compress=True)

loaded_model = joblib.load("MY_MODEL.pkl")
loaded_model.load_nn_model("MY_MODEL")

The NeuralModel save and load methods should be refactored (save all the relevant attributes and make load a class method) to fix the bug.

However, the current NeuralModel class is far from current Deep Learning standards (ex: PyTorch Lightning modules) and could be refactored entirely.
In particular, we could separate the model itself from the trainer class (Training attributes are unnecessary when doing inference).

@HugoPerrier
Copy link
Contributor

New melusine v3.0.0 is out.
Closing legacy issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants