-
Notifications
You must be signed in to change notification settings - Fork 126
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
Updated tracker and eval to compute timely accuracy. #160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracking implementation looks good besides minor fixing. It would be worth adding a docstring comment explaining that the tracker implementation only covers the case when the tracker runtime < frame gap.
There's also the issue about anchoring (not all frames' ground truths are evaluated on), which I think we should address
self._csv_logger.info('{},{},{},{},{}'.format( | ||
time_epoch_ms(), sim_time, self.config.name, 'runtime', | ||
runtime)) | ||
# Write metrics to csv log file | ||
self.__write_metrics_to_csv(metrics_summary_df, sim_time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless of the anchoring (start or finish) we should avoid using sim_time because it's neither: if the queue gets backed up multiple (start, end) inferences will be processed and logged with the same sim_time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes and issues found from unit testing tracker.
@@ -75,26 +79,53 @@ def on_watermark(self, timestamp, obstacle_tracking_stream): | |||
frame_msg = self._frame_msgs.popleft() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If all streams send a watermark.is_top at the very end (after a watermark for every message is sent), when the watermark.is_top is sent this function will get invoked, and pop_left will raise an error because the buffer is empty.
To get around this we should first explicitly agree on a universal procedure for sending watermark.is_top (either set the watermark coming after the last message to be is_top or sending a separate watermark.is_top message afterwards). If we do the latter then we can add an if statement before the frame_msg.popleft that forwards the top watermark and returns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's assume that there's a special top watermark, that comes last. I'll add an if statement to the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments built up looking at code
metrics_summary_df = self.__get_tracker_metrics( | ||
tracker_obstacles, ground_obstacles, | ||
self._end_anchroned_accumulator) | ||
self.__write_metrics_to_csv(metrics_summary_df, ground_time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.__write_metrics_to_csv(metrics_summary_df, ground_time) | |
self.__write_metrics_to_csv(metrics_summary_df, "end", ground_time) |
No description provided.