Added upper limit of the instance count in prediction score labeling suggestion method #981
Conversation
…_score Corrected formatting issues
There was a problem hiding this comment.
A small correction that for an edge case and then we can also add some tests in the tests/gui/test_suggestions.py file. I wrote a sample test that we can add to:
def test_limits_prediction_score(centered_pair_predictions: Labels):
"""Testing suggestion generation using instance limits and prediction score."""
labels = centered_pair_predictions
score_limit = 20
instance_lower_limit = 3
instance_upper_limit = 3
# Generate suggestions
suggestions = VideoFrameSuggestions.suggest(
labels=labels,
params={
"videos": labels.videos,
"method": "prediction_score",
"score_limit": score_limit,
"instance_limit_upper": instance_upper_limit,
"instance_limit_lower": instance_lower_limit,
},
)
# Confirming every suggested frame meets criteria
for sugg in suggestions:
lf = labels.get((sugg.video, sugg.frame_idx))
pred_instances = [
inst for inst in lf.instances_to_show if isinstance(inst, PredictedInstance)
]
n_instance_below_score = np.nansum(
[True for inst in pred_instances if inst.score < score_limit]
)
assert n_instance_below_score >= instance_lower_limit
assert n_instance_below_score <= instance_upper_limitThe limits are set score limit is set s.t. we only get 6 suggestions returned (for speed of the assertion loop at the end).
sleap/gui/suggestions.py
Outdated
| # Finds non-zero entries in idxs | ||
| result = sorted(idxs[idxs > 0].tolist()) |
There was a problem hiding this comment.
Consider case when we want to return the first frame (frame indices start at 0). The current implementation ensures that the first frame will never be returned despite having valid criteria.
| # Finds non-zero entries in idxs | |
| result = sorted(idxs[idxs > 0].tolist()) | |
| # Finds non-negative entries in idxs | |
| result = sorted(idxs[idxs >= 0].tolist()) |
There was a problem hiding this comment.
Changed accordingly! Thank you!
sleap/gui/suggestions.py
Outdated
| ): | ||
| lfs = labels.find(video) | ||
| frames = len(lfs) | ||
| idxs = np.ndarray((frames), dtype="int") | ||
| scores = np.full((frames, instance_limit), 100.0, dtype="float") | ||
| idxs = np.zeros((frames), dtype="int") |
There was a problem hiding this comment.
| idxs = np.zeros((frames), dtype="int") | |
| idxs = np.full((frames), -1, dtype="int") |
We never expect to see a negative frame index, but we might see a frame index of 0.
environment.yml
Outdated
| @@ -18,6 +18,7 @@ dependencies: | |||
| # - sleap::pyside2=5.14.1 | |||
| - qtpy>=2.0.1 | |||
| - conda-forge::pip!=22.0.4 | |||
| - pip | |||
There was a problem hiding this comment.
Did you experience any issues with this removed?
There was a problem hiding this comment.
Old yaml gave infinite warnings suggesting some packages require pip, without pip installation cannot be done. Adding pip solved the problem, so was environment_build.yml, but the built-in environment_m1.yml has pip line already.
|
I have corrected the comments in suggestions.py . Also added the sample test from @roomrys to confirm all the suggested frames meet the criteria (corrected |
Codecov Report
@@ Coverage Diff @@
## develop #981 +/- ##
===========================================
- Coverage 66.77% 66.77% -0.01%
===========================================
Files 127 127
Lines 21494 21493 -1
===========================================
- Hits 14352 14351 -1
Misses 7142 7142
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
tests/gui/test_suggestions.py
Outdated
| for video in labels.videos: | ||
| lfs = labels.find(video) |
There was a problem hiding this comment.
We can remove this outer for loop as there is only a single video in the centered_pair_predictions fixture.
| for video in labels.videos: | |
| lfs = labels.find(video) | |
| lfs = labels.labeled_frames |
There was a problem hiding this comment.
Done! Also corrected another mistake I made earlier
| n_qualified_instance = np.nansum(frame_scores <= score_limit) | ||
|
|
||
| if ( | ||
| n_qualified_instance >= instance_limit_lower |
There was a problem hiding this comment.
instance_limit_lower has a default of 0, which will evaluate to True for empty frames I think. Like:
frame_scores = np.array([]) # == no predicted instances in frame
n_qualified_instance = np.nansum(frame_scores <= score_limit) # == 0Do we maybe want the lower limit to be 1 or switch to using > instead of >=?
There was a problem hiding this comment.
That's a good point. I guess one of the uses of this feature is to quickly locate the frames where prediction failed, and that would include empty frames (e.g. after a whole video prediction). Therefore, I prefer to keep the option open to users
There was a problem hiding this comment.
Fair enough! Feel free to approve after CI passes @roomrys
|
Regarding to @talmo 's point earlier, I realized that setting lower limit to 0 will also give out frames with user instances and no predicted instances, which is likely not desirable if user only wants to get frames failed in prediction. A quick workaround is to consider all user instances to be score 100, and include/exclude them by tweaking score limit (100 = user + pred; 99 = pred only). A more ideal solution is to give user an option to narrow the searching scope to unlabeled frames. This could be a dropdown menu like |
Description
PR 2/3 for #977
Renamed original instance num input to
instance_limit_lower. Addedinstance_limit_upperto the method. Updated suggestion config file accordingly. User can now have finer control over the number of instances in generating prediction score based labeling suggestion.I found it useful to quickly generate frame list with only certain number of visible predicted instances. I am working on a 2-animal project, having 1 instance means failure on the other animal.
This can be achieved by a small tweak on <prediction_score> suggestion method. I renamed original input <num_instance> to <instance_at_least> added another input <instance_no_more_than>. Now I can just set <predition_score> to 100 and both instance upperbound and lowerbound to be 1, and label suggestion will give back all frames with precisely 1 predicted instance. This obviously enables other frame filtering possibilities as well.
Originally posted by @hsingchien in #977
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️