Skip to content
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

Draw bbox #118

Merged
merged 7 commits into from
Nov 3, 2023
Merged

Draw bbox #118

merged 7 commits into from
Nov 3, 2023

Conversation

MateoLostanlen
Copy link
Member

The PR to display the prediction the firefighters wanted!

I've added a button to toggle the prediction on or off for a better view in the case of a small fire.

I've removed the question "Is the alert relevant?", which is redundant with the question below, to free up space.

I'm going to update the devices with the latest version of engine so that they all send the prediction.

Screenshot from 2023-10-30 11-55-13

Screenshot from 2023-10-30 11-55-19

@MateoLostanlen
Copy link
Member Author

We have a bandit error because of try catch in loop for update_live_alerts_data call back I propose to manage it out of this PR

Copy link
Collaborator

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @MateoLostanlen, thanks a lot for this PR, overall everything seems OK.

Sometimes there is no bbox on detection frames (screenshot bellow), could you explain why ?
Also in this case should we still display the frame as the ticked option seems to be confusing.

Capture d’écran 2023-10-31 à 09 37 11

Comment on lines 296 to 303
individual_alert_frame_placeholder_children = []
for event_id, frame_url_list in images_url_live_alerts.items():
individual_alert_frame_placeholder_children.append(
html.Div(
id={"type": "individual_alert_frame_storage", "index": str(event_id)},
children=frame_url_list,
style={"display": "none"},
)
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True that it's actually never used, but this means that this Callback is actually creating the div

@MateoLostanlen
Copy link
Member Author

hello @Akilditu ,

Pyro-engine combines 4 frames to send an alert in order to reduce FP rate. The box we display is the prediction of the model on each frame, if the model do not detect anything there is no box

Copy link
Collaborator

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also small thought when you delete code please try to let some comments about defs so that we can understand the purpose of it.

I know they're quite long but but do not delete everything 😅

app/main.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MateoLostanlen everything seems fine now, one last thing : I understand the 4 frames needed for Pyro Engine but should we still display frames on which there is no box / detection ?
I understand that this would somehow break the 'detection flow' that shows the fire evolution but I find it kinda weird to have random frames with and without boxes in between. Feels a bit like the model is messing things up ..

@MateoLostanlen
Copy link
Member Author

@Akilditu I understand your concern about the consistency of the detection flow. However, it's important to consider that every frame, whether it has a detected box or not, contains valuable information about the presence or absence of fire at a given moment. Removing frames without detections could potentially discard subtle cues or indicators that are crucial for a comprehensive understanding of the fire's evolution.

Firefighters rely on the full context of an evolving situation. Even frames where the AI does not detect fire can serve as negative data points that contribute to the overall pattern of the event. These 'empty' frames can act as confirmation that the fire has not spread to certain areas at specific times, which is just as important as knowing where the fire is active.

Therefore, keeping all frames ensures that firefighters get the complete picture, which is essential for their safety and the effectiveness of their response. It is better to err on the side of providing too much information rather than too little in such critical applications.

@Akilditu Akilditu self-requested a review November 3, 2023 11:29
Akilditu
Akilditu previously approved these changes Nov 3, 2023
Copy link
Collaborator

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good then, let's wait for user feedbacks about detection frames without bbox and this interpolation solution we may have !

@MateoLostanlen MateoLostanlen merged commit 2b70b31 into main Nov 3, 2023
7 of 8 checks passed
@MateoLostanlen MateoLostanlen deleted the draw_bbox branch November 3, 2023 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants