feat(deepeval): add run_async method to DeepEvalEvaluator - #3695
Conversation
Closes deepset-ai#3667. Implements run_async using each metric's a_measure method, evaluating test cases concurrently. A separate metric copy is created per test case because DeepEval metrics keep score/reason state on the instance. Tests cover all supported metrics and mirror the existing sync output assertions.
|
Hi @RahilOp, thanks for your interest in contributing to Haystack! 🙏 This is an automated message to help us keep the review queue healthy. |
|
Heads-up for maintainers This PR is from a fork and touches integrations whose integration tests require API keys. Affected integrations:
Please run the integration tests locally ( |
Coverage report (deepeval)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Signed-off-by: Syed Ali Abbas Rahil <viperboom0786110@gmail.com>
| ``a_measure`` method. A separate metric copy is used per test case | ||
| because DeepEval metrics keep state (``score``, ``reason``) on the |
There was a problem hiding this comment.
Don't use double backticks for docstrings
| ``a_measure`` method. A separate metric copy is used per test case | |
| because DeepEval metrics keep state (``score``, ``reason``) on the | |
| `a_measure` method. A separate metric copy is used per test case | |
| because DeepEval metrics keep state (`score`, `reason`) on the |
| :returns: | ||
| A dictionary with a single `results` entry that contains | ||
| a nested list of metric results. The shape matches the | ||
| output of :meth:`run`. |
There was a problem hiding this comment.
| output of :meth:`run`. | |
| output of the `run` method |
| assert got == expected | ||
|
|
||
|
|
||
| def test_invoke_deepeval(monkeypatch): |
There was a problem hiding this comment.
Could you explain why we need this test? It seems like its covered by existing tests
| # This integration test validates the evaluator by running it against the | ||
| # OpenAI API. It is parameterized by the metric, the inputs to the evalutor | ||
| # and the metric parameters. |
There was a problem hiding this comment.
Please add an integration test for the async route and please test it yourself locally
|
|
||
| @staticmethod | ||
| async def _invoke_deepeval_async(test_cases: list[LLMTestCase], metric: BaseMetric) -> EvaluationResult: | ||
| """Evaluate ``test_cases`` concurrently using the metric's ``a_measure``.""" |
There was a problem hiding this comment.
remove the double back ticks in the docstrings
| retrieval_context=cast(list[str] | None, test_case.retrieval_context), | ||
| ) | ||
|
|
||
| results = await asyncio.gather(*[_evaluate_one(tc) for tc in test_cases]) |
There was a problem hiding this comment.
This line is unbounded with the number of concurrent requests that we make. Please cap it by using Sempahore and sensible default for number of concurrent requests.
Related Issues
Proposed Changes:
run_asynctoDeepEvalEvaluatorso it can be used inPipeline.run_async()without blocking the caller's loop.a_measuremethod and evaluates test cases concurrently withasyncio.gather.deepeval.metrics.utils.copy_metrics, because DeepEval metrics keep result state (score,reason) on the metric instance._convert_resultsso bothrunandrun_asyncshare the same output-formatting logic.How did you test it?
cd integrations/deepeval hatch run test:unit -v hatch run test:types hatch run fmt-checkNotes for the reviewer
This follows the same pattern used by
RagasEvaluator.run_asyncin this repo and by the publica_measureAPI suggested in the issue discussion. It intentionally does not bump thedeepevalpin (>=2.9.0remains supported).