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

feat: add local evaluation support using LlamaCppChatGenerator #7745

Closed
wants to merge 7 commits into from

Conversation

lbux
Copy link
Contributor

@lbux lbux commented May 25, 2024

Related Issues

Proposed Changes:

This implements a WIP version of local evaluation support with the feedback received in the respective issue.

During my work on this, there were some changes to the files, so I fixed the conflicts and merged. I will need to go over those changes more in depth and see if it makes sense to implement some of the changes for the llama.cpp version.

How did you test it?

Added tests (copies of OpenAI tests slightly modified for llama cpp implementation.

Notes for the reviewer

The implementation will require anyone using llm_evaluator to install llama-cpp-python which might not be ideal, but I don't see a way around it (lazy import best solution?)

There is a TODO in the code that I will need to figure out. Ideally, I would like to be able to extract whether the specific model supports the system prompt from Llama, but that doesn't seem easy. My current thought is to allow an optional flag and default to supporting system prompt when no flag is provided.

The link to LlamaCppChatGenerator does not work as it is not live yet.

Installing a model is necessary to run the tests. I added logic to handle that the same way we handle it in the llama cpp generator.

Checklist

@lbux lbux requested a review from a team as a code owner May 25, 2024 04:04
@lbux lbux requested review from anakin87 and removed request for a team May 25, 2024 04:04
@CLAassistant
Copy link

CLAassistant commented May 25, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the type:documentation Improvements on the docs label May 25, 2024
@lbux
Copy link
Contributor Author

lbux commented May 25, 2024

It seems like DynamicChatPromptBuilder is being deprecated. I will need to update the generator and this implementation as well.

@shadeMe shadeMe self-requested a review May 27, 2024 07:46
@shadeMe
Copy link
Collaborator

shadeMe commented Jun 3, 2024

Thanks for working on this! Could you fix the code so that the CI tests pass?

@lbux
Copy link
Contributor Author

lbux commented Jun 6, 2024

The previous CI fails were due to llama-cpp-python and llama-cpp-haystack not being in the hatch env for testing. I added them and it should fix it, but on my end I'm still failing this test:
FAILED test/test_imports.py::test_for_missing_dependencies - AssertionError: Module haystack_integrations is not in the dependencies

This is one of those things that I brought up on the discord about this being maybe not the right approach due to coupling (and also now the main haystack repository relies on a package from haystack-core-components).

@anakin87 anakin87 removed their request for review June 14, 2024 10:12
@masci masci changed the title feat[evaluation]: add local evaluation support using LlamaCppChatGenerator feat: add local evaluation support using LlamaCppChatGenerator Jun 18, 2024
@lbux
Copy link
Contributor Author

lbux commented Jun 25, 2024

I'm going to close this as I don't think this is the proper way to integrate local evaluation support with the current implementation of LLMEvaluator. If there ever exists a way to connect a generator to an evaluator without coupling, I can implement support for llama.cpp for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants