mmpose icon indicating copy to clipboard operation
mmpose copied to clipboard

Clarification on min_keypoints in tracking

Open pallgeuer opened this issue 3 years ago • 2 comments
trafficstars

Could someone explain the motivation behind this seemingly arbitrary line of code? https://github.com/open-mmlab/mmpose/blob/c8e91ff456d82c7f985bf938cabfb68b2aa51d27/mmpose/apis/inference_tracking.py#L227

Here is a copy of the relevant section with the line marked:

        if track_id == -1:
            if np.count_nonzero(result['keypoints'][:, 1]) >= min_keypoints:
                result['track_id'] = next_id
                next_id += 1
            else:
                # If the number of keypoints detected is small,
                # delete that person instance.
                result['keypoints'][:, 1] = -10  # <-- This line
                result['bbox'] *= 0
                result['track_id'] = -1

Zeroing out the bounding box and setting the tracking ID to the reserved invalid value of -1 I can understand. But why is it appropriate to set the y-value of all keypoints to an arbitrary value of -10?

Wouldn't it be more appropriate to do:

result['keypoints'][:, 2] = 0

Or even:

result['keypoints'].fill(0)

?

Also, what is stopping the function from actually deleting result from results? Otherwise most pieces of code that use the results of tracking has to start by filtering out the invalid results.

@ly015 You seem to be the author of the commit that added this line of code: https://github.com/open-mmlab/mmpose/commit/d56465a09cec4acaa2fdabc2e7847c79730f9f2d

pallgeuer avatar Jun 03 '22 14:06 pallgeuer

Thank you very much for pointing out this issue. The author has given some explanation on this line, but I agree that this can be handled in a better way like you suggest. We will refactor this part soon. image

ly015 avatar Jun 06 '22 03:06 ly015

Okay thanks.

If it's only about pushing it off the screen so that it isn't visualised then I think deleting the result from results really would be better, otherwise as I said every piece of code that tries to iterate through the results has to be wary of these (undocumented?) "fake" results and manually filter them out on every use. Also, I'm not sure that this doesn't affect OKS tracking, as it is still hypothetically possible for the off-the-screen keypoints to still get matched right? (very unlikely but I don't think it's impossible)

Anyway, while this is a fix for bottom up models, it isn't a fix for top-down models. When I use one of the two top-down models trained on Human3.6M on non-Human3.6M data, I get detections involving only 1-3 valid keypoints, yet the min_keypoints check doesn't trigger because all 17 keypoints have non-zero coordinates and scores, just with scores under the keypoint score threshold. Hence the min_keypoints check fails to identify this case.

pallgeuer avatar Jun 07 '22 08:06 pallgeuer