fix(io): reject unusable frame numbers and unscorable ground truth - #561
Draft
AlexBodner wants to merge 1 commit into
Draft
fix(io): reject unusable frame numbers and unscorable ground truth#561AlexBodner wants to merge 1 commit into
AlexBodner wants to merge 1 commit into
Conversation
Two ways MOT input was accepted and then silently produced wrong numbers.
`load_mot_file` took any parsable frame number. Every consumer walks
frames with `range(1, num_frames + 1)`, so rows on frame 0 or a negative
frame were loaded and then never evaluated, and `int(float(...))` quietly
truncated a fractional frame onto a different frame. Both are now
rejected, naming the file and the offending row. Frames written as floats
with no fractional part ("1.0") stay valid.
`evaluate_mot_sequence` scored ground truth that filtered down to nothing
as a clean run of zeros, which is indistinguishable from a tracker that
found nothing. It now raises, pointing at the ground-truth file.
The most common way to hit this is reusing tracker output as ground
truth: tracker files record class -1, and only pedestrian-class rows are
scored. The writer is left alone, since TrackEval tracker files carry no
class either — the error explains the mismatch instead.
The guard lives in `evaluate_mot_sequence` rather than
`_prepare_mot_sequence` because an individual all-distractor frame is
legitimate, and existing tests pin that primitive's TrackEval-mirroring
behavior of returning zero scored rows.
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens MOT evaluation input validation to prevent silently incorrect metrics.
Changes:
- Rejects non-positive and fractional frame numbers.
- Rejects ground truth with no scorable pedestrian rows and adds regression tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/trackers/io/mot.py |
Validates frame numbers. |
src/trackers/eval/evaluate.py |
Rejects unscorable ground truth. |
tests/io/test_mot.py |
Tests frame validation. |
tests/eval/test_evaluate.py |
Tests ground-truth filtering errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+211
to
+218
| frame_number = float(row[0]) | ||
| except ValueError as e: | ||
| raise ValueError(f"Invalid frame number in {path}: {row[0]}") from e | ||
|
|
||
| # Consumers walk frames with `range(1, num_frames + 1)`, so anything outside that | ||
| # domain would be loaded here and then silently never evaluated. | ||
| if not frame_number.is_integer(): | ||
| raise ValueError(f"Frame numbers must be whole numbers in {path}, got {row[0]} in row: {row}") |
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.
Problem
Two ways MOT input is accepted and then silently produces wrong numbers.
Frame numbers.
load_mot_fileaccepts any parsable frame number. Every consumer walks frames withrange(1, num_frames + 1), so rows on frame 0 or a negative frame are loaded and then never evaluated.int(float(...))also quietly truncates a fractional frame onto a different frame.Unscorable ground truth. Only pedestrian-class (1) rows marked for consideration are scored, mirroring TrackEval. Ground truth where every row fails that filter evaluates to a clean run of zeros — indistinguishable from a tracker that found nothing.
The common way to hit the second one: reusing tracker output as ground truth. Tracker files record class
-1, so every row is filtered out and the whole benchmark reports zeros.Fix
load_mot_filerejects frame numbers that are not whole numbers greater than zero, naming the file and the offending row. Frames written as floats with no fractional part (1.0) stay valid.evaluate_mot_sequenceraises when ground truth has no scored rows, pointing at the file and explaining the class filter.Tests
0,-3and1.7are rejected;1.0still loads; the error names the file.tests/io,tests/evalandtests/tunepass at 141.Decisions worth review
The writer is left alone.
_MOTOutputkeeps writing class-1, because TrackEval tracker files carry no class either — the writer is correct and the error explains the mismatch. The alternative was emitting class1so round-tripping silently works; that seemed worse than a clear error. Easy to flip if you disagree.The guard is in
evaluate_mot_sequence, not_prepare_mot_sequence. An individual all-distractor frame is legitimate, and existing tests pin the primitive's TrackEval-mirroring behaviour of returning zero scored rows. Raising there would have broken that contract.Made with Cursor