-
Notifications
You must be signed in to change notification settings - Fork 0
KLAUS-256: Adding filters for gt data in compare-gt.py #44
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
base: master
Are you sure you want to change the base?
Conversation
michberger
left a comment
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 works well and is a very helpful tool for testing the performance of classifiers on the ground truth dataset. Only suggestion is that the filter and stitch values used be indicated on the output ethogram and bout performance figures for reference.
| filter_scan, | ||
| iou_thresholds, | ||
| filter_ground_truth, | ||
| False, |
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.
@SkepticRaven, I'm curious about your opinion on permanently setting this to False. This is thefilter_ground_truth argument of the generate_iou_scan call.
It's used to switch on this call to filter_by_settings.
Which, for reference, is defined here.
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.
From a machine learning perspective, we should avoid modifying the ground truth data. I'd prefer it wasn't a parameter at all. The reason this capability exists is just for practical reasons. While the user should go in and manually modify the ground truth data, it can get effort-expensive. Allowing the ground truth to be modify-able runs the risk of observing "improved" performance by removing shorter but difficult real events.
At least in this edit, the behavior appears to be generating an unfiltered and filtered version (edits below, L345).
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.
So does it make sense to remove the filter_ground_truth argument entirely?
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.
We should first look into what generate_filtered_iou_curve does differently. That new function appears to have high overlap with what that arg does.
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.
I think it'd make more sense for me to change this back to filter_ground_truth and modify my code to work with some of the older logic. In the current state, I rewrote the whole function without removing the older functionality.
Occasionally, there are cases where the user may want to filter their own gt annotations as well as the predictions. I have added logic that allows the user to do this. The filtering values will be the same for both gt and pred. The user has to specify these values to also help prevent a user from accidentally using this information.
Maybe in the future iteration I will also add an ethogram output that fully does not include the raw data.
Please test this feature request using another dataset! I have only managed to test it on Feeding behavior. Hopefully @michberger can find the time to review & test.