Feature/litellm

[clementou]

Closes #29

📑 Description

Obtains chain with LiteLLM to support more API's.

✅ Checks

• [x] My pull request adheres to the code style of this project
• [ ] My code requires changes to the documentation
• [ ] I have updated the documentation as required
• [x] All the tests have passed
• [x] Branch name follows type/descript (e.g. feature/add-llm-agents)
• [x] Ready for code review

ℹ Additional Information

[ProKil]

Hi @clementou thanks for contributing to sotopia-lab/sotopia!

Overall I think this PR makes sense and does a simple and concrete thing.

The only problem is the backward compatibility of this PR. Is this a breaking change that would disable some code running before, or an add-on change which enables LiteLLM but doesn't break anything?

I think the former one makes me more comfortable to approve this PR. Nice job!

[clementou]

By backwards compatible do you mean something like a model name that worked before might not work anymore? In that case, it'll still work. LiteLLM just sits where the OpenAI API or the TogetherAI API used to sit.

You can also setup aliases in LiteLLM so for example if you want to use Llama, it'll automatically change it to the togethercomputer version of llama.

I also think it might be better to remove the LLM_Name literal completely and replace references to it with str, delegating validation of model names to LiteLLM. However, it looks like its used for type checking in a lot of places I don't fully understand the purpose of.

[XuhuiZhou]

Sorry for the delay in feedback @clementou!

I think it's always a good practice to enable back-compatibility? Is there any technical difficulty if we want to keep LLM_Name?

My understanding of LLM_Name is that it strictly requires a certain string that minimizes the chances of making mistakes/reproducibility issues.

But defs feel free to share ur thoughts here @ProKil

[clementou]

Right now it doesn't actively use LLM_Name to check anything for experiments but I think it's still used for type checking and some tests when you commit.

If you want to actively use it for checking strings I don't think it's possible with LiteLLM, since that just restricts the models you can use. LiteLLM will already throw an error if a model name is unrecognized, so don't think there is a need to validate it twice.

[ProKil]

Ok as confirmed by members on other projects. It should easy to adapt to this breaking change.

I approve this PR