@alimaredia FYI

@dhellmann Note this will remove the fastchat dependency

@dhellmann Note this will remove the fastchat dependency

Ack, thanks for the heads-up.

@danmcp are we confident we don't need to rely on fastchat for any future developments to the code that we are importing from them?
This seems great from a dependency reduction point of view, but I'm guessing we would essentially be "freezing" the borrowed code (unless we plan to develop it in a different direction)

@danmcp are we confident we don't need to rely on fastchat for any future developments to the code that we are importing from them?

I wish I could predict the future and answer this question :). We had already included the bulk of the fastchat code we needed within eval. This change largely includes the remainder of customizations from xukai92

lm-sys/FastChat@main...xukai92:FastChat:ilab

that aren't included in FastChat anyway and leaves out support for most model type conversations. Given the amount of customization we are doing already, and the amount the code is tied to OpenAI vs FastChat, I think we are stuck maintaining our own version that works with the versions of OpenAI and vllm and models we need to support.

I think the biggest risk comes to changes we need to make to the evaluation conversation with the model and judge and less to the core code.

This seems great from a dependency reduction point of view, but I'm guessing we would essentially be "freezing" the borrowed code (unless we plan to develop it in a different direction)

There is already a decent bit of divergence with xukai92's changes, adding support for a new OpenAI version, support for custom judges, and support for merge-system-user-message.

This is definitely a good question to ask. This particular PR doesn't increase the risk very much vs what we had already done, but the overall problem of how to maintain everything long term still exists.

@Mergifyio refresh

refresh

✅ Pull request refreshed

This pull request has merge conflicts that must be resolved before it can be
merged. @danmcp please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork