FastChat icon indicating copy to clipboard operation
FastChat copied to clipboard

Decouple LLM Interface Code for Improved Scalability

Open real-limitless opened this issue 2 years ago • 1 comments

Hello,

I've noticed that in the FastChat/fastchat/conversation.py file has code that interfaces with multiple LLMs.

I think that as we continue to add more LLMs, there will come a time when we need to decouple the code necessary to interact with each LLM for better organization and scalability.

Currently, the get_default_conv_template() function contains numerous if statements to determine the corresponding LLM conversation, as shown below:

def get_default_conv_template(model_name):
    ...
    # A series of if statements for each LLM
    ...
    return ret.copy()

As support for more languages and models increase, I suggest that each LLM type should have its own Python file with all its corresponding conversations and modules inside.

This way, our codebase will be better organized and more scalable.

Here's a list of the current LLMs:

  • baize
  • conv_one_shot
  • dolly
  • koala_v1
  • oasst
  • stablelm
  • vicuna_v1.1
  • rwkv
  • buddy

I propose creating separate files for each LLM, such as baize.py, conv_one_shot.py, dolly.py, etc., containing their respective conversation templates and any other necessary code.

This would allow us to avoid using a long list of if statements and make it easier to maintain and extend the codebase.

Please let me know your thoughts on this proposal, and if you have any suggestions for improvements or alternatives.

Thanks!

real-limitless avatar May 07 '23 23:05 real-limitless

This is a good point and I also noticed this. I did some refactoring in #1008. Could you take a look and review it? I am not sure whether it is the best design so I would love to hear your feedback.

Now for a new model, supporting it would require adding two self-contained code block without "if"

A conversation template https://github.com/lm-sys/FastChat/blob/68d1fdcc311af987173bcfa49ffcdff9d5120365/fastchat/conversation.py#L258-L267

A model adapter https://github.com/lm-sys/FastChat/blob/68d1fdcc311af987173bcfa49ffcdff9d5120365/fastchat/model/model_adapter.py#L266-L280

merrymercy avatar May 08 '23 04:05 merrymercy

This is a good point and I also noticed this. I did some refactoring in #1008. Could you take a look and review it? I am not sure whether it is the best design so I would love to hear your feedback.

Now for a new model, supporting it would require adding two self-contained code block without "if"

A conversation template

https://github.com/lm-sys/FastChat/blob/68d1fdcc311af987173bcfa49ffcdff9d5120365/fastchat/conversation.py#L258-L267

A model adapter

https://github.com/lm-sys/FastChat/blob/68d1fdcc311af987173bcfa49ffcdff9d5120365/fastchat/model/model_adapter.py#L266-L280

This modification is perfect, Very easy to understand.

Thank you,

real-limitless avatar May 15 '23 02:05 real-limitless