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.

Sign up for GitHub

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

Jump to bottom

Changed transformers_client design so it can support most (all?) HuggingFace models #208

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Changed transformers_client design so it can support most (all?) HuggingFace models #208

wants to merge 36 commits into from

Conversation

adjahossoualexandre
Copy link

New design

TransformerClient into 3 client classes:

  1. TransformerEmbeddingModelClient
  • Merger of TransformerEmbedder and TransformerClient
  1. TransformerRerankerModelClient
  • Merger of TransformerReranker and TransformerClient
  1. TransformerLLMModelClient
  • Merger of TransformerLLM and TransformerClient

Pros:

  • Abstracts model/tokenizer initialisation as well as inference code
  • Hence, most of the time, the only customization needed is to change some parameters during client instantiation
  • Works with many different model architectures (example here)
  • Make the code easier to read and more maintainable since each class takes care of only one client
  • Easier to customize for users as it removes the need for model SDKs
  • Removes the need to check whether a model is supported or not

Cons:

  • The tradeoff here would be to couple inference code with the response and api kwargs handling.

@adjahossoualexandre
DRAFT: merge TransformerClient & TransformerEmbedder into 1 class.
e8a690b
@adjahossoualexandre
Fixed typo.
123396c
@adjahossoualexandre
Added type hints to signatures + removed now useless model_type.
2121774
@adjahossoualexandre
Removed now useless model_types.
e2023b2
@adjahossoualexandre
Added test for TransformerEmbeddingModelClient execution.
a93bd6a
@adjahossoualexandre
Changed my mind.
424cbfb
@adjahossoualexandre
Changed my mind. removing model type might introduce issues.
ef2783d
@adjahossoualexandre
Removed now useless argument.
3601445
@adjahossoualexandre
Removed now useless arguments.
3cdab7b
@adjahossoualexandre
DRAFT: merge TransformerClient and TransformerLLM in 1 class.
2d1152f
@adjahossoualexandre
Multiline message:
d839eda 
Added 'tokenizer_kwargs' in TransformerLLMModelClient constructor for more flexibility.

Added chat template argument in constructor for more flexibility.

Added pad token check.

Added tokenizer in '_infer_from_pipeline()' when chat_template is used (required).

Fixed _handle_input() for 'apply_chat_template'==True.

Not sure: ficed message in convert_inputs_to_api_kwargs().
@adjahossoualexandre
Added ests for TransformerLLMModelClient.
7daf8ae
@adjahossoualexandre
Removed temporary log.
a4eb3bb
@adjahossoualexandre
Changed my mind: added model_type back into call().
6a9c657
@adjahossoualexandre
Changed my mind about model type. See prev commit.
cd1823b
@adjahossoualexandre
Ensured tokenizer_kwargs has 'return_tensors' set to 'pt' by default.
2bf711a
@adjahossoualexandre
DRAFT: merge TransformerClient and TransformerReranker in 1 class.
570c8b1
@adjahossoualexandre
Commented out old classes.
881cfb6
@adjahossoualexandre
Added tests for TransformerRerankerModelClient.
bd610d2
@adjahossoualexandre
Add test for llm response + remove test for old class.
25fe834
@adjahossoualexandre
Fixed test class name.
e3b3b25
@adjahossoualexandre
Multiline message:
2c6ee8c 
Moved get_device andclean_device_cache at top of file.

Allow user to specify autoclasses for  Reranker models.
@adjahossoualexandre
Deleted code for the old TransformerClientClass.
1bd8545
@adjahossoualexandre
Added __doc__ for the client classes.
0059bbf
@adjahossoualexandre
Formatting.
53d5384
@adjahossoualexandre
Added example for transformers_client module + fixed import.
be8041c
@adjahossoualexandre
Restored originial file.
f4abfeb
@adjahossoualexandre
Fixed test class name.
39f2ae2
@adjahossoualexandre
Added kwargs for model and tokenizer init.
5481ee7
@adjahossoualexandre
Fixed typo.
eab39cd
@liyin2015
Copy link
Member

@adjahossoualexandre Thank you for the effort, this is a big change, we will have to review a bit later!

@adjahossoualexandre
Copy link
Author

@adjahossoualexandre Thank you for the effort, this is a big change, we will have to review a bit later!

Sure, let me know if you need me to explain some parts of the code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adjahossoualexandre @liyin2015