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

fix(perception_online_evaluator): add metric_value not only stat (#7100)(#7118) (revert of revert) #7167

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

kosuke55
Copy link
Contributor

@kosuke55 kosuke55 commented May 30, 2024

this PR need to wait for driving log replayer

Reverts #7118

#7100

Description

Previously, only stat with max, mean, and min as keys could be handled in diagnostics, but this PR allows metric_value with a single value.
This is to allow only one value to be included in metrics such as object counts, since all stat values are the same, which is redundant.

  • stat type
- level: "\0"
  name: predicted_path_deviation_variance_CAR_5.00
  message: ''
  hardware_id: ''
  values:
  - key: min
    value: '0.039821'
  - key: max
    value: '0.039821'
  - key: mean
    value: '0.039821'
  • metric_value type
- level: "\0"
  name: interval_average_objects_count_UNKNOWN_r100.00_h10.00
  message: ''
  hardware_id: ''
  values:
  - key: metric_value
    value: '4.000000'

image

Tests performed

psim

Effects on system behavior

Not applicable.

Interface changes

no interface changes for autoware.

but the diagnostic key used for tools (driving_log_replayer etc) is chaged

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@github-actions github-actions bot added the component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) label May 30, 2024
@kosuke55 kosuke55 marked this pull request as ready for review May 30, 2024 01:55
@kosuke55 kosuke55 added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label May 30, 2024
@kosuke55 kosuke55 requested a review from hayato-m126 May 30, 2024 03:07
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

Attention: Patch coverage is 64.51613% with 11 lines in your changes missing coverage. Please review.

Project coverage is 15.58%. Comparing base (e800d54) to head (a3efa8a).
Report is 24 commits behind head on main.

Current head a3efa8a differs from pull request most recent head 10366f4

Please upload reports for the commit 10366f4 to get more accurate results.

Files Patch % Lines
...evaluator/src/perception_online_evaluator_node.cpp 66.66% 0 Missing and 6 partials ⚠️
...eption_online_evaluator/src/metrics_calculator.cpp 40.00% 0 Missing and 3 partials ⚠️
...tor/test/test_perception_online_evaluator_node.cpp 75.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7167      +/-   ##
==========================================
+ Coverage   14.87%   15.58%   +0.70%     
==========================================
  Files        1992     1979      -13     
  Lines      138800   137069    -1731     
  Branches    43714    44458     +744     
==========================================
+ Hits        20653    21361     +708     
+ Misses      95359    92351    -3008     
- Partials    22788    23357     +569     
Flag Coverage Δ *Carryforward flag
differential 50.58% <64.51%> (?)
total 15.57% <ø> (+0.69%) ⬆️ Carriedforward from be9a2b8

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kosuke55 and others added 4 commits June 7, 2024 17:35
@kosuke55 kosuke55 force-pushed the revert-7118-revert-7100-feat/metric_value branch from d4edf4c to 10366f4 Compare June 7, 2024 08:35
Copy link
Contributor

@hayato-m126 hayato-m126 left a comment

Choose a reason for hiding this comment

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

LGTM

I confirmed that the following two types of DianosticsStatus are produced by actually running the code.

This PR code works with tier4/driving_log_replayer#467

@hayato-m126 hayato-m126 merged commit 4577719 into main Jun 7, 2024
23 of 25 checks passed
@hayato-m126 hayato-m126 deleted the revert-7118-revert-7100-feat/metric_value branch June 7, 2024 09:23
KhalilSelyan pushed a commit that referenced this pull request Jul 22, 2024
…)(#7118) (revert of revert) (#7167)

* Revert "fix(perception_online_evaluator): revert "add metric_value not only s…"

This reverts commit d827b1b.

* Update evaluator/perception_online_evaluator/include/perception_online_evaluator/perception_online_evaluator_node.hpp

* Update evaluator/perception_online_evaluator/src/perception_online_evaluator_node.cpp

* use emplace back

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

---------

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Co-authored-by: Kotaro Uetake <60615504+ktro2828@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants