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

fix: support embedding with LiteLLM for Ragas #56

Merged
merged 7 commits into from
Dec 5, 2024
Merged

fix: support embedding with LiteLLM for Ragas #56

merged 7 commits into from
Dec 5, 2024

Conversation

undo76
Copy link
Contributor

@undo76 undo76 commented Dec 4, 2024

Use the configured embedder with Ragas

@undo76 undo76 requested a review from lsorber December 4, 2024 16:34
lsorber
lsorber requested changes Dec 4, 2024
View reviewed changes
Copy link
Member

@lsorber lsorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments.

src/raglite/_eval.py Outdated Show resolved Hide resolved
src/raglite/_eval.py Outdated Show resolved Hide resolved
src/raglite/_eval.py Show resolved Hide resolved
src/raglite/_eval.py Show resolved Hide resolved
src/raglite/_eval.py Outdated Show resolved Hide resolved
src/raglite/_eval.py Show resolved Hide resolved
src/raglite/_eval.py Outdated Show resolved Hide resolved
@undo76 @lsorber
Update src/raglite/_eval.py
ef108af 
Co-authored-by: Laurent Sorber <[email protected]>
@lsorber
Copy link
Member

lsorber commented Dec 4, 2024

Unrelated to the review, but would you mind if we turn this into a fix:? You could argue RAGLite should have supported any embedding model to begin with. Also, I don't want to race minor version number bumps if we can avoid it.

undo76 and others added 3 commits December 4, 2024 17:52
@undo76 @lsorber
Update src/raglite/_eval.py
a59dc32 
Co-authored-by: Laurent Sorber <[email protected]>
@undo76 @lsorber
Update src/raglite/_eval.py
d91b52a 
Co-authored-by: Laurent Sorber <[email protected]>
@undo76 @lsorber
Update src/raglite/_eval.py
e42ac00 
Co-authored-by: Laurent Sorber <[email protected]>
@undo76 undo76 changed the title feat: Use any embedding model with ragas fix: Use any embedding model with ragas Dec 4, 2024
lsorber
lsorber approved these changes Dec 5, 2024
View reviewed changes
Copy link
Member

@lsorber lsorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be merged! But before we do, could you quickly confirm you tested this locally?

@undo76
Copy link
Contributor Author

undo76 commented Dec 5, 2024

yes, it was tested and Ragas working with OpenAI as embedder.

@lsorber lsorber changed the title fix: Use any embedding model with ragas fix: support embedding with LiteLLM for Ragas Dec 5, 2024
@lsorber lsorber merged commit f6023f5 into main Dec 5, 2024
2 checks passed
@lsorber lsorber deleted the feat/ms-ragas-embeddings branch December 5, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsorber lsorber lsorber approved these changes

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
@undo76 @lsorber