minor bug fixes#163
Draft
omri374 wants to merge 5 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses character-based IoU calculation issues in span matching evaluation by fixing an off-by-one error and adjusting the matching threshold for more flexible span matching.
- Fixed off-by-one error in character span calculation by removing unnecessary
+ 1in range calls for both annotation and prediction end indices - Reduced the default IoU threshold from 0.9 to 0.75 to allow more flexibility in span matching
- Added comprehensive tests to verify the fix for split entity scenarios and exact matches
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| presidio_evaluator/evaluation/span_evaluator.py | Updated IoU threshold default to 0.75 and fixed off-by-one error in character-based IoU calculations by making end indices properly exclusive |
| tests/test_span_evaluator.py | Added two new test cases to verify character-based IoU calculations work correctly for split entities and exact matches |
| presidio_evaluator/evaluation/scorers.py | Removed obsolete scoring pipeline code that is no longer needed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| generic_entities: Optional[List[str]] = None, | ||
| skip_words: Optional[List] = None, | ||
| iou_threshold: float = 0.9, | ||
| iou_threshold: float = 0.75, |
There was a problem hiding this comment.
The documentation comment at line 35 states the default iou_threshold is 0.5, but the actual default has been changed to 0.75. The documentation should be updated to reflect this change:
:param iou_threshold: Minimum Intersection over Union (IoU) threshold for considering spans as matching.
Value between 0 and 1, where higher values require more overlap (default: 0.75)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the evaluation logic for span matching, focusing on improving the accuracy of character-based IoU calculations and adjusting the threshold for matches. It also adds new tests to ensure correct behavior, especially for split entities. The most important changes are grouped below:
Evaluation Logic Improvements
iou_thresholdinSpanEvaluatorfrom0.9to0.75to allow more flexibility in span matching._calculate_combined_iouby removing the unnecessary+ 1in therangecalls for both annotation and prediction spans. This ensures that character indices are properly exclusive at the end, improving the accuracy of IoU calculations. [1] [2]Testing Enhancements
test_span_evaluator.py:test_calculate_combined_iou_char_based_split_entityverifies that split predictions covering an annotation result in an IoU of 1.0, confirming the off-by-one bug is fixed.test_calculate_combined_iou_char_based_exact_matchchecks that exact matches yield an IoU of 1.0, ensuring no regression in the basic matching logic.