Split detections #539
Split detections #539
Conversation
src/app/models.py
Outdated
| sequence_id: Union[int, None] = Field(None, foreign_key="sequences.id", nullable=True) | ||
| azimuth: float = Field(..., ge=0, lt=360) | ||
| bucket_key: str | ||
| bboxes: str = Field(..., min_length=2, max_length=settings.MAX_BBOX_STR_LENGTH, nullable=False) |
There was a problem hiding this comment.
To be clearer, we should rename this field as bbox in the singular (and update all references to it).
src/app/models.py
Outdated
| azimuth: float = Field(..., ge=0, lt=360) | ||
| bucket_key: str | ||
| bboxes: str = Field(..., min_length=2, max_length=settings.MAX_BBOX_STR_LENGTH, nullable=False) | ||
| others_bboxes: Union[str, None] = Field(default=None, nullable=True) |
There was a problem hiding this comment.
Should have a max lenght as well derived from max_length=settings.MAX_BBOX_STR_LENGTH (2 bboxes max in other bboxes with the current logic)
| ) | ||
| matched_sequence: Optional[Sequence] = None | ||
| for seq in candidate_sequences: | ||
| if seq.id is None: |
There was a problem hiding this comment.
How can it be empty ? candidate_sequences are coming from a sequences fetch_all, hence, objects are supposed to have an id.
| azimuth = detection_data.get("azimuth") | ||
| if azimuth is None: | ||
| azimuth = detection_data.get("pose_azimuth") | ||
| if azimuth is None: |
There was a problem hiding this comment.
I think we need to reconsider which info to provide in slack message. This sequence of if statements with different concepts emphasises that the information provided is unclear.
fe51
left a comment
fe51
left a comment
There was a problem hiding this comment.
Made commits to fixed some discussed points.
Should be challenged once you are back, but thanks a lot for this feature !
What’s new in this PR
Detection model and API changes
Dropped
Detection.azimuthMade
pose_idmandatory for all detectionsCreation logic now:
others_bboxesSequencing logic improvements
Sequences are now looked up using:
camera_idpose_idlast_seenFor each new detection:
Its bbox is parsed
A candidate sequence is reused only if the last bbox overlaps
Otherwise, a new sequence is created
This prevents merging detections that are spatially distant but temporally close
Helpers refactor
Why this matters
The main goal of this change is to separate unrelated detections coming from the same image.
When a single frame contains both a real fire and a false positive, these detections have nothing in common spatially or semantically and must not end up in the same sequence.
By enforcing one detection per bbox and reusing sequences only when spatial overlap exists, we ensure that fires and false positives evolve independently, even if they originate from the same image and timestamp