Fixing #5254 #5267
Fixing #5254 #5267
Conversation
WalkthroughThe changes made in this pull request focus on the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluation.tsx (1)Pattern 🔇 Additional comments (6)fiftyone/operators/builtins/panels/model_evaluation/__init__.py (4)
The method follows the single responsibility principle and provides a clear way to identify binary classification evaluations.
The calculation of true positives, false positives, and false negatives for binary classification is correctly implemented using numpy operations.
The code correctly handles the special case for binary classification by using uppercase field values and appropriate matching conditions.
Incorrect calculation of object detection metrics The current implementation of TP/FP/FN for object detection might be inaccurate:
Apply this fix: - tp_count = np.count_nonzero(results.ytrue == results.ypred)
- fp_count = np.count_nonzero(results.ytrue == results.missing)
- fn_count = np.count_nonzero(results.ypred == results.missing)
+ tp_count = np.count_nonzero(
+ (results.ytrue != results.missing) &
+ (results.ypred != results.missing) &
+ (results.ytrue == results.ypred)
+ )
+ fp_count = np.count_nonzero(
+ (results.ytrue == results.missing) &
+ (results.ypred != results.missing)
+ )
+ fn_count = np.count_nonzero(
+ (results.ytrue != results.missing) &
+ (results.ypred == results.missing)
+ )app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluation.tsx (2)Line range hint The addition of Line range hint The updated hide conditions for TP/FP/FN metrics correctly use the new Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fiftyone/operators/builtins/panels/model_evaluation/__init__.py(3 hunks)
🔇 Additional comments (1)
fiftyone/operators/builtins/panels/model_evaluation/__init__.py (1)
305-306: Handle cases where get_tp_fp_fn returns None values
The get_tp_fp_fn method may return None for tp, fp, and fn when the evaluation type is unsupported. When assigning these to metrics["tp"], metrics["fp"], and metrics["fn"], please ensure that the downstream code can handle None values appropriately. Consider adding checks or default values to prevent potential issues.
| if info.config.type == "detection": | ||
| tp_count = np.count_nonzero(results.ytrue == results.ypred) | ||
| fp_count = np.count_nonzero(results.ytrue == results.missing) | ||
| fn_count = np.count_nonzero(results.ypred == results.missing) | ||
| return tp_count, fp_count, fn_count |
There was a problem hiding this comment.
Possible miscalculation of false positives and false negatives in object detection
In the object detection case, the calculation of fp_count and fn_count might be incorrect. The fp_count should count cases where ypred is present but ytrue is missing (i.e., predictions with no corresponding ground truth). Similarly, fn_count should count cases where ytrue is present but ypred is missing (i.e., ground truth objects missed by predictions). The current implementation counts fp_count as np.count_nonzero(results.ytrue == results.missing), which does not consider whether ypred is present.
To address this issue, consider modifying the calculations as follows:
def get_tp_fp_fn(self, info, results):
# Object detection
if info.config.type == "detection":
- tp_count = np.count_nonzero(results.ytrue == results.ypred)
- fp_count = np.count_nonzero(results.ytrue == results.missing)
- fn_count = np.count_nonzero(results.ypred == results.missing)
+ tp_count = np.count_nonzero(
+ (results.ytrue != results.missing) &
+ (results.ypred != results.missing) &
+ (results.ytrue == results.ypred)
+ )
+ fp_count = np.count_nonzero(
+ (results.ytrue == results.missing) &
+ (results.ypred != results.missing)
+ )
+ fn_count = np.count_nonzero(
+ (results.ytrue != results.missing) &
+ (results.ypred == results.missing)
+ )
return tp_count, fp_count, fn_count📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if info.config.type == "detection": | |
| tp_count = np.count_nonzero(results.ytrue == results.ypred) | |
| fp_count = np.count_nonzero(results.ytrue == results.missing) | |
| fn_count = np.count_nonzero(results.ypred == results.missing) | |
| return tp_count, fp_count, fn_count | |
| if info.config.type == "detection": | |
| tp_count = np.count_nonzero( | |
| (results.ytrue != results.missing) & | |
| (results.ypred != results.missing) & | |
| (results.ytrue == results.ypred) | |
| ) | |
| fp_count = np.count_nonzero( | |
| (results.ytrue == results.missing) & | |
| (results.ypred != results.missing) | |
| ) | |
| fn_count = np.count_nonzero( | |
| (results.ytrue != results.missing) & | |
| (results.ypred == results.missing) | |
| ) | |
| return tp_count, fp_count, fn_count |
Resolves #5254
Summary by CodeRabbit
New Features
Bug Fixes
Documentation