-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
refactor: Generate eval result in separate method #5001
Conversation
Pull Request Test Coverage Report for Build 5067539591
💛 - Coveralls |
custom_document_id_field: Optional[str] = None, | ||
context_matching_min_length: int = 100, | ||
context_matching_boost_split_overlaps: bool = True, | ||
context_matching_threshold: float = 65.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity - where did these defaults like context_matching_threshold
and their values come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They come from eval_batch
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
context_matching_boost_split_overlaps: bool = True, | ||
context_matching_threshold: float = 65.0, | ||
use_auth_token: Optional[Union[str, bool]] = None, | ||
) -> EvaluationResult: | ||
eval_result = EvaluationResult() | ||
if add_isolated_node_eval: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would need to happen before we do the batch evaluation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bogdankostic This causes isolated evaluation to be omitted
Related Issues
Proposed Changes:
This PR creates a separate method for generating eval results from batch predictions.
How did you test it?
Manual tests + CI
Notes for the reviewer
This is needed for benchmarking, as we want to get performance metrics for Pipelines but want to measure pure inference time without the overhead of calculating the metrics.
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.