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

Unified Model Interfaces #567

Closed
ixxie opened this issue Feb 25, 2019 · 3 comments
Closed

Unified Model Interfaces #567

ixxie opened this issue Feb 25, 2019 · 3 comments
Labels
wontfix This will not be worked on

Comments

@ixxie
Copy link

ixxie commented Feb 25, 2019

Is your feature/enhancement request related to a problem? Please describe.

Flair is a great library in that it provides a uniform interface for disparate NLP models in one place. So far I see two main weaknesses in the library in this respect:

  1. Some things, like serialization/deserialization of the models and caching of downloaded models do not really have explicit interfaces in all models, which means at best I am left to my own devices in implementing a solution and at worst that I have to fight the library for control over these processes.
  2. Some of the existing interfaces are different across different models, in particular Embedding is very different from the other models. SequenceTagger for example has a .predict method and Embeddings have .embed methods.

Describe the solution you'd like

My suggestion is that the model class hierarchy would be refactored in a way that there is a new universal (abstract) base class for the whole library. It would include uniform interfaces with methods like:

  • .predict - incremental prediction
  • .train - incremental training
  • .train_predict - incremental training and prediction
  • .batch - training and/or prediction for a batch of files
  • .fetch - fetch pretrained model from the web
  • .load - deserialize model
  • .save - serialize model

Doing this would make it easier to combine the models and abstract over them. It would make it easy to do:

for model in models:
    for sentence in sentences:
        model.predict(sentence)

for any list of Flair models. Such a move would make it easy to write a FlairModel which allows the user to instantiate a model with a mix of various language model(s), tagger(s), embedder(s) and classifier(s) suitably stacked and linked. This class would help me seamlessly combine all of these into a single NLP model, the complete lifecycle of which I could control completely using a combined interface. Thus calling flairmodel.train(sentence) would train the all the taggers, embeddings, and other models I put inside the model, simultaneously.

It is true embedding models are different than sequence tagging or text classification models, but they are still a type of supervised model so the terminology makes sense to me at least. It does seem that in Flair most embeddings are not trainable though, so I can see why at the moment it would seem weird to use train/predict terminology. It would be easy enough to maintain .embed as an alias to .predict for embedding models.

Additional context

Perhaps I am biased by my previous work with Gensim; there we have a sprawling mess of interfaces which are all different, and forced my team to write wrappers to make the interface uniform enough to make the models fit standardized processes. I believe making models more interchangeable helps both during model selection and model deployment phases of a project.

I see Flair as potentially replacing Gensim in many contexts in the future; it might become a sort of Scikit-Learn for NLP even. Designing a uniform interface at this point would help it scale elegantly as it acquired more and more models.

@ixxie ixxie mentioned this issue Feb 25, 2019
5 tasks
@alanakbik
Copy link
Collaborator

@ixxie interesting idea! From my side, I would argue that only things that perform the same function need to inherit from the same abstract base class. So, all embeddings should inherit from an embedding class, and all models should inherit from a base model class. But since their function in the code is different, they do not need to inherit from the same base class as this could lead to confusion and may introduce errors. Still, I think this is an interesting idea and I'd be happy to explore this further!

@ixxie
Copy link
Author

ixxie commented Feb 25, 2019

My reasoning here is that while these models differ in many significant ways, there are a few things they share:

  • they are all supervised ML models, so they have distinct training and prediction steps, even though the nature of their prediction is vastly different.
  • they all have pretrained models that need to be fetched from the internet (usually).
  • they can all be serialized and deserialized.

Since these are also the most common steps the user takes, it helps that they are named the same for all models.

The beauty of a class hierarchy is that the differences can be accentuated as subclasses are derived from parent classes. As such, the specialized functionality and method aliases (such as .embed as a .predict alias for Embedding classes) could be added at the parent class for those particular models. As such I think not much is lost in terms of the current interfaces; this might even be implementable in a completely backwards compatible way!

I think I outlined the benefits already (helps the user implement and maintain the library, opens the door to a FlairModel class) so I will rest my case. Perhaps others can chime in, this may also be a matter of taste.

@stale
Copy link

stale bot commented Apr 30, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Apr 30, 2020
@stale stale bot closed this as completed May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants