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

add eval functions #124

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

add eval functions #124

wants to merge 13 commits into from

Conversation

ysli16
Copy link
Contributor

@ysli16 ysli16 commented Oct 8, 2024

Add common functions to evaluate the performance of ego vehicle in the simulation.
For safety, the functions to find the minimum distance, minimum time-to-collision(ttc) and maximum deceleration-rate-to-avoid-collision(drac) are implemented. The vehicle geometry is considered in the computation.
For efficiency, the function to find the first time that the ego vehicle entered its desired lane is implemented. It only checks whether the vehicle enters the goal lane(or its predecessor/successor), regardless of the progress on the lane.
For comfort, the max jerk and the rms of frequency-weighted acceleration(according to ISO2631) is computed.

This will be merged after tested locally for the pdm4ar exercise.

@alezana
Copy link
Member

alezana commented Oct 8, 2024

@ysli16 could you please

@alezana
Copy link
Member

alezana commented Oct 8, 2024

Other suggestions from looking at the code:

  • max jerk can be rewritten in a more compact way with iterate with dt
  • use list/tuple comprehension if you can (more compact and usually slightly more efficient)
    E.g.
acc_time = []
    for idx in range(len(commands.timestamps)):
        acc_time.append(commands.values[idx].acc)

can be a single line acc_time = [v.acc for v in commands.values]

  • avoid ranges to iterate over iterables if you then need to extract values (more error prone and less elegant)
    E.g.
    for idx in range(0, len(states.values)):
        state = states.values[idx]
        timestep = states.timestamps[idx]
        reached = desired_lane_reached(lanelet_network, goal_lane, state, pos_tol, heading_tol)
        if reached:
            reached_time = timestep
            break
    return float(reached_time)

can be

    for idx, state in enumerate(states.values):
        reached = desired_lane_reached(lanelet_network, goal_lane, state, pos_tol, heading_tol)
        if reached:
            reached_time = states.timestamps[idx]
            break
    return float(reached_time)

ps: Here I don't really like the -1 as return if it is not reached, very bug prone

@ysli16 ysli16 requested a review from alezana October 8, 2024 10:42
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 84.37500% with 40 lines in your changes missing coverage. Please review.

Project coverage is 80.22%. Comparing base (21d9670) to head (d2d0553).

Files with missing lines Patch % Lines
src/dg_commons/eval/efficiency.py 62.66% 19 Missing and 9 partials ⚠️
src/dg_commons/eval/safety.py 94.59% 3 Missing and 5 partials ⚠️
src/dg_commons/eval/comfort.py 87.87% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #124      +/-   ##
==========================================
+ Coverage   78.31%   80.22%   +1.91%     
==========================================
  Files          72       75       +3     
  Lines        4943     5199     +256     
  Branches      331      377      +46     
==========================================
+ Hits         3871     4171     +300     
+ Misses        991      930      -61     
- Partials       81       98      +17     

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

@alezana alezana marked this pull request as ready for review October 8, 2024 17:48
@alezana
Copy link
Member

alezana commented Oct 8, 2024

@ysli16 see the above comment as review

Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Some food for thought. View full project report here.

src/dg_commons_tests/test_eval/test_safety_eval.py Outdated Show resolved Hide resolved
src/dg_commons_tests/test_eval/test_safety_eval.py Outdated Show resolved Hide resolved
src/dg_commons_tests/test_eval/test_safety_eval.py Outdated Show resolved Hide resolved
src/dg_commons_tests/test_eval/test_safety_eval.py Outdated Show resolved Hide resolved
src/dg_commons_tests/test_eval/test_safety_eval.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants