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

Improved steering accuracy measurement script #23583

Merged
merged 3 commits into from
May 12, 2022

Conversation

jyoung8607
Copy link
Collaborator

@jyoung8607 jyoung8607 commented Jan 20, 2022

This script clearly hasn't been used in awhile. Made some improvements while I was fixing it up.

  • Fix breakage from several moved/deprecated cereal fields
  • Support all current lateral control methods (I think)
  • Start collecting on active, not enabled
  • Don't collect during ALC, to ignore the brief steering nudge before the steering-pressed threshold is met
  • Add lateral control saturation reporting for each angle
  • Segregate reporting results by speed group (chosen somewhat arbitrarily by me)

Example output:

COLLECTING ...

speed group: fast                25 - 35 m/s  //  90 - 126 km/h  //  56 - 78 mph
  ------------------------------------------------------------------------------
  angle:  0 | error: 0.15 | =: 20% | +:  43% | -: 36% | sat:    0 | count:  1672
  angle:  1 | error: 0.10 | =: 32% | +:  40% | -: 26% | sat:    0 | count:  4959
  angle:  2 | error: 0.11 | =: 24% | +:  22% | -: 53% | sat:    0 | count:  3643
  angle:  3 | error: 0.14 | =: 26% | +:  18% | -: 55% | sat:    0 | count:   679
  angle:  4 | error: 0.23 | =: 15% | +:   0% | -: 84% | sat:    0 | count:   240

speed group: medium               15 - 25 m/s  //  54 - 90 km/h  //  34 - 56 mph
  ------------------------------------------------------------------------------
  angle:  0 | error: 0.15 | =: 20% | +:  45% | -: 33% | sat:    0 | count: 10785
  angle:  1 | error: 0.18 | =: 17% | +:  19% | -: 62% | sat:    0 | count: 11444
  angle:  2 | error: 0.27 | =: 11% | +:   7% | -: 80% | sat:    0 | count:  4009
  angle:  3 | error: 0.44 | =:  1% | +:   8% | -: 90% | sat:    0 | count:   622
  angle:  4 | error: 0.69 | =:  1% | +:  27% | -: 71% | sat:    0 | count:   176
  angle:  5 | error: 0.53 | =:  0% | +:  37% | -: 62% | sat:    0 | count:   156
  angle:  6 | error: 0.66 | =:  8% | +:   9% | -: 82% | sat:    0 | count:   312
  angle:  7 | error: 0.65 | =:  6% | +:  19% | -: 74% | sat:    0 | count:   210
  angle:  8 | error: 0.60 | =:  0% | +:   0% | -:100% | sat:    0 | count:   489

speed group: slow                  5 - 15 m/s  //  18 - 54 km/h  //  11 - 34 mph
  ------------------------------------------------------------------------------
  angle:  0 | error: 0.15 | =: 14% | +:  56% | -: 28% | sat:    0 | count:  1046
  angle:  1 | error: 0.15 | =: 23% | +:  30% | -: 45% | sat:    0 | count:  1047
  angle:  2 | error: 0.15 | =: 14% | +:  30% | -: 54% | sat:    0 | count:   265

speed group: crawl                    0 - 5 m/s  //  0 - 18 km/h  //  0 - 11 mph
  ------------------------------------------------------------------------------
  angle:  0 | error: 0.16 | =: 10% | +:  40% | -: 48% | sat:    0 | count:   332
  angle:  1 | error: 0.14 | =: 21% | +:   8% | -: 70% | sat:    0 | count:   648
  angle:  2 | error: 0.47 | =:  0% | +:   0% | -:100% | sat:    0 | count:    15

This made it useful for me, but waiting for replay to run is lame. I may follow up with an option to use LogReader.

@jyoung8607 jyoung8607 marked this pull request as draft January 20, 2022 03:11
@jyoung8607
Copy link
Collaborator Author

I added an average for absolute actuators.steer, the fraction of total output torque used for non-angle controls, though I'm taking it before car port rate limiting. I also added an average for absolute lateralPlan.dPathPoints. I'm not quite sure how it's defined or what its unit of measure is, but it seems to show how far off we are from the planned path. It's not a measure of steering accuracy, but it might help us understand driving stability and quality as we approach controllable maximums.

speed group: fast                                                 25 - 35 m/s  //  90 - 126 km/h  //  56 - 78 mph
  ---------------------------------------------------------------------------------------------------------------
   0 | actuator: 11% | error: 0.15 | =: 20% | +:  43% | -: 36% | sat cnt:    0 | dpathpoints: 0.03 | total:  1672
   1 | actuator: 12% | error: 0.10 | =: 32% | +:  40% | -: 26% | sat cnt:    0 | dpathpoints: 0.02 | total:  4960
   2 | actuator: 26% | error: 0.11 | =: 24% | +:  22% | -: 53% | sat cnt:    0 | dpathpoints: 0.03 | total:  3647
   3 | actuator: 35% | error: 0.14 | =: 26% | +:  18% | -: 55% | sat cnt:    0 | dpathpoints: 0.05 | total:   679
   4 | actuator: 50% | error: 0.23 | =: 15% | +:   0% | -: 84% | sat cnt:    0 | dpathpoints: 0.07 | total:   240

speed group: medium                                                15 - 25 m/s  //  54 - 90 km/h  //  34 - 56 mph
  ---------------------------------------------------------------------------------------------------------------
   0 | actuator:  7% | error: 0.14 | =: 24% | +:  39% | -: 36% | sat cnt:    0 | dpathpoints: 0.02 | total:  3709
   1 | actuator: 14% | error: 0.19 | =: 18% | +:  19% | -: 62% | sat cnt:    0 | dpathpoints: 0.04 | total:  3451
   2 | actuator: 26% | error: 0.29 | =: 13% | +:   9% | -: 77% | sat cnt:    0 | dpathpoints: 0.06 | total:  1377
   3 | actuator: 38% | error: 0.44 | =:  1% | +:   9% | -: 89% | sat cnt:    0 | dpathpoints: 0.11 | total:   275
   4 | actuator: 58% | error: 0.79 | =:  2% | +:  30% | -: 67% | sat cnt:    0 | dpathpoints: 0.11 | total:    49
   5 | actuator: 41% | error: 0.57 | =:  0% | +:  60% | -: 39% | sat cnt:    0 | dpathpoints: 0.21 | total:    23
   6 | actuator: 74% | error: 1.40 | =: 10% | +:  10% | -: 78% | sat cnt:    0 | dpathpoints: 0.11 | total:    46
   7 | actuator: 69% | error: 0.79 | =:  9% | +:   0% | -: 90% | sat cnt:    0 | dpathpoints: 0.08 | total:    65
   8 | actuator: 77% | error: 0.69 | =:  0% | +:   0% | -:100% | sat cnt:    0 | dpathpoints: 0.09 | total:   175

@adeebshihadeh
Copy link
Contributor

Let's put this in a new directory called tools/tuning/. Then we can throw other docs and tuning scripts in there too.

@jyoung8607
Copy link
Collaborator Author

jyoung8607 commented Jan 20, 2022

Will do. Also planning some other rework after chatting with Harald, but I'm out of time for OP stuff this week.

Notes to self:

  • move into tools/tuning
  • get steer from actuatorsOutput instead of actuators
  • report limited (car port safety rate limiting) count as well as saturation count
  • remove dPathPoints (interesting but unpredictable and nonrepeatable for this purpose)
  • add comparison of plan curvature to vehicle model curvature
  • make it work with LogReader as well as live/replay, or perhaps instead-of

speed_group_stats[group][angle_abs]["dpp"] += abs(d_path_points[0])
if control_state.saturated:
speed_group_stats[group][angle_abs]["saturated"] += 1
if actual_angle == desired_angle:
Copy link
Contributor

@ClockeNessMnstr ClockeNessMnstr Jan 21, 2022

Choose a reason for hiding this comment

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

Suggested change
if actual_angle == desired_angle:
if angle_error <= .5:

Edit: angle error less than half a degree makes a pretty useful bin

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't think I follow. Both actual_angle and desired_angle are rounded to one decimal point, so the comparison should work as desired. Only angle_abs is rounded to int, and that only determines which bucket the stats go in.

Copy link
Contributor

@ClockeNessMnstr ClockeNessMnstr Jan 24, 2022

Choose a reason for hiding this comment

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

I misread the precision. So:
1.04 =/= 1.06 but .96 == 1.04.

Copy link
Contributor

Choose a reason for hiding this comment

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

So angle_error <= .05, would be more consistent.

Copy link
Collaborator Author

@jyoung8607 jyoung8607 Jan 24, 2022

Choose a reason for hiding this comment

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

I don't think we'll have any angle_error <= 0.05 because I think (except maybe ZSS-upgraded Toyota?) we have no vehicles with steering angle precisions better than 0.1 degree. The error will either be zero, or >= 0.1 degrees.

Copy link
Contributor

@ClockeNessMnstr ClockeNessMnstr Jan 24, 2022

Choose a reason for hiding this comment

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

Ah, fair enough. Although that begs the question, why even have the "==" column.

Something like <= .5 would make the == column a more meaningful bin/stat.

With <=.5, Output like below is much more useful as the over/under are actually measuring a more meaningful value as well.

   0° | actuator: 24% | error: 0.51° | -: 12%:- | =: 58%:= | +: 28%:+ | sat cnt:    0 | dppnts: 0.14 | total:   714
   1° | actuator: 19% | error: 0.40° | -: 11%:- | =: 70%:= | +: 17%:+ | sat cnt:    0 | dppnts: 0.09 | total:  1865
   2° | actuator: 18% | error: 0.39° | -: 20%:- | =: 76%:= | +:  3%:+ | sat cnt:    0 | dppnts: 0.08 | total:  2156
   3° | actuator: 30% | error: 0.62° | -: 63%:- | =: 33%:= | +:  3%:+ | sat cnt:    0 | dppnts: 0.08 | total:   737

Copy link
Contributor

Choose a reason for hiding this comment

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

Desired angle is not limited by precision, so angle error is rarely zero, right?

Copy link
Contributor

@ClockeNessMnstr ClockeNessMnstr Jan 26, 2022

Choose a reason for hiding this comment

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

It's rounded here so the match will "work" but it's still rarely zero just because it's so restrictive.

Making the comparison right at the limit of resolution also means that rounding error is significant. Something like 25% error although it should average out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hehe, yes angle_error <= 0.05 because I think (except maybe ZSS-upgraded Toyota?) ZSS goes down to something like 0.0000001 😂

@ClockeNessMnstr
Copy link
Contributor

ClockeNessMnstr commented Jan 21, 2022

Was playing around with this. found a useful argument to add for myself at least.
Added a number of rows to print argument since it overflowed the display with lines with a small amount of data.
It finds the (n+1)th largest number of data points and only prints lines with more data points than that.
(prints up to n lines, plus any matching group headers, but removes a tie for nth place.)

import heapq
--------
parser.add_argument('--rows', default='999', help="maximum number of data lines to print")
--------
  try:
   row_limit = int(args.rows)
  except Exception:
    raise ValueError("please enter a number for argument: rows")
--------    
    if msg_cnt % 100 == 0:
      pivotq = []
      num_lines = 0
      for group in display_groups:
        if len(speed_group_stats[group]) > 0:
          for k in sorted(speed_group_stats[group].keys()):
            v = speed_group_stats[group][k]
            heapq.heappush(pivotq,v["cnt"])
            num_lines +=1
      if num_lines > row_limit:
        pivot = heapq.nlargest(row_limit + 1, pivotq)[-1]
      else:
        pivot = 0

      print(chr(27) + "[2J")
      for group in display_groups:
        if len(speed_group_stats[group]) > 0:
          header = False
          for k in sorted(speed_group_stats[group].keys()):
            v = speed_group_stats[group][k]
            if v["cnt"] > pivot + 1:
              header = True
          if header:
            print(f"  speed group: {group:10s} {all_groups[group][1]:>91s}")
            print(f"  {'-'*113}")
            for k in sorted(speed_group_stats[group].keys()):
              v = speed_group_stats[group][k]
              if v["cnt"] > pivot + 1:
                print(f'  {k:#2}° | actuator:{int(v["steer"] / v["cnt"] * 100):#3}% | error: {round(v["err"] / v["cnt"], 2):2.2f}° | -:{int(v["-"] / v["cnt"] * 100):#3}%:- | =:{int(v["="] / v["cnt"] * 100):#3}%:= | +:{int(v["+"] / v["cnt"] * 100):#3}%:+ | sat cnt: {v["saturated"]:#4} | dppnts: {round(v["dpp"] / v["cnt"], 2):2.2f} | total: {v["cnt"]:#5}'#) broke this just in case, not part of the Github comment
          print("")
      if cnt != 0:
        print(" COLLECTING ...\n")
      else:
        print(" DISABLED (not active, standstill, steering override, or lane change)\n")
speed group: fast                                                    30 - 35 m/s  // 108 - 126 km/h //  67 - 78 mph
  -----------------------------------------------------------------------------------------------------------------
   0° | actuator: 17% | error: 0.27° | -:  3%:- | =: 95%:= | +:  0%:+ | sat cnt:    0 | dppnts: 0.10 | total:   835
   1° | actuator: 17% | error: 0.35° | -: 10%:- | =: 71%:= | +: 18%:+ | sat cnt:    0 | dppnts: 0.09 | total:   582
   2° | actuator: 14% | error: 0.32° | -: 11%:- | =: 80%:= | +:  7%:+ | sat cnt:    0 | dppnts: 0.09 | total:   434
   3° | actuator: 18% | error: 0.12° | -:  0%:- | =: 98%:= | +:  0%:+ | sat cnt:    0 | dppnts: 0.09 | total:  1122
   4° | actuator: 43% | error: 0.46° | -: 45%:- | =: 54%:= | +:  0%:+ | sat cnt:    0 | dppnts: 0.05 | total:    55

speed group: medium                                                  15 - 20 m/s  //  54 - 72 km/h  //  34 - 45 mph
  -----------------------------------------------------------------------------------------------------------------
   0° | actuator: 24% | error: 0.51° | -: 12%:- | =: 58%:= | +: 28%:+ | sat cnt:    0 | dppnts: 0.14 | total:   714
   1° | actuator: 19% | error: 0.40° | -: 11%:- | =: 70%:= | +: 17%:+ | sat cnt:    0 | dppnts: 0.09 | total:  1865
   2° | actuator: 18% | error: 0.39° | -: 20%:- | =: 76%:= | +:  3%:+ | sat cnt:    0 | dppnts: 0.08 | total:  2156
   3° | actuator: 30% | error: 0.62° | -: 63%:- | =: 33%:= | +:  3%:+ | sat cnt:    0 | dppnts: 0.08 | total:   737
   4° | actuator: 43% | error: 1.37° | -: 96%:- | =:  1%:= | +:  1%:+ | sat cnt:    0 | dppnts: 0.14 | total:   102
   5° | actuator: 48% | error: 1.77° | -: 92%:- | =:  0%:= | +:  7%:+ | sat cnt:    0 | dppnts: 0.12 | total:    69
   7° | actuator: 83% | error: 2.62° | -: 64%:- | =: 17%:= | +: 17%:+ | sat cnt:    0 | dppnts: 0.19 | total:    28
  21° | actuator: 38% | error: 1.40° | -: 45%:- | =: 45%:= | +:  9%:+ | sat cnt:    0 | dppnts: 0.39 | total:    51
  22° | actuator: 77% | error: 1.50° | -: 39%:- | =: 59%:= | +:  1%:+ | sat cnt:    0 | dppnts: 0.45 | total:    61
  23° | actuator: 78% | error: 2.20° | -:100%:- | =:  0%:= | +:  0%:+ | sat cnt:    4 | dppnts: 0.43 | total:    68
  24° | actuator: 96% | error: 3.38° | -:100%:- | =:  0%:= | +:  0%:+ | sat cnt:   31 | dppnts: 0.37 | total:    61

speed group: slow                                                    10 - 15 m/s  //  36 - 54 km/h  //  22 - 34 mph
  -----------------------------------------------------------------------------------------------------------------
   2° | actuator: 24% | error: 0.17° | -:  1%:- | =: 98%:= | +:  0%:+ | sat cnt:    0 | dppnts: 0.07 | total:    74
   3° | actuator:  8% | error: 0.39° | -: 17%:- | =: 82%:= | +:  0%:+ | sat cnt:    0 | dppnts: 0.02 | total:    63

DISABLED (not active, standstill, steering override, or lane change)

@sshane
Copy link
Contributor

sshane commented Apr 19, 2022

git merge --no-ff upstream/master ;)

@jyoung8607
Copy link
Collaborator Author

jyoung8607 commented Apr 19, 2022

Ready for consideration.

There are more things I'd like to do with this script, like make it take a route instead of require replay, stop trying to combine negative and positive steering angles so we can look at road crown effects properly, and add comparison of curvatures instead of just steering angles. However, this is a good-enough checkpoint and I've verified it works on routes from current master. Looking forward to feedback and contributions from others interested in tuning.

@jyoung8607 jyoung8607 marked this pull request as ready for review April 19, 2022 19:49
@jyoung8607
Copy link
Collaborator Author

jyoung8607 commented Apr 19, 2022

For lack of a better place to share/commemorate it, here's an interesting result from @Verylukyguy:

speed group: fast                                                        25 - 35 m/s  //  90 - 126 km/h  //  56 - 78 mph
  ----------------------------------------------------------------------------------------------------------------------
   0° | actuator: 18% | error: 0.41° | -: 58% | =:  7% | +: 34% | lim: 3670 | sat:    0 | path dev: 0.05m | total: 23746
   1° | actuator: 28% | error: 0.70° | -: 82% | =:  4% | +: 12% | lim: 4254 | sat:    0 | path dev: 0.05m | total: 25553
   2° | actuator: 44% | error: 1.06° | -: 84% | =:  1% | +: 14% | lim: 2439 | sat:    0 | path dev: 0.07m | total: 13062
   3° | actuator: 40% | error: 0.84° | -: 51% | =:  2% | +: 46% | lim:  791 | sat:    0 | path dev: 0.06m | total:  3332
   4° | actuator: 46% | error: 0.59° | -: 45% | =:  5% | +: 49% | lim:  714 | sat:    0 | path dev: 0.05m | total:  3643
   5° | actuator: 56% | error: 0.59° | -: 52% | =:  4% | +: 42% | lim:  557 | sat:    0 | path dev: 0.05m | total:  2970
   6° | actuator: 68% | error: 0.75° | -: 62% | =:  2% | +: 34% | lim:  494 | sat:    0 | path dev: 0.04m | total:  2475
   7° | actuator: 79% | error: 0.77° | -: 76% | =:  3% | +: 19% | lim:  224 | sat:    0 | path dev: 0.05m | total:  1172
   8° | actuator: 85% | error: 0.69° | -: 72% | =:  6% | +: 21% | lim:  159 | sat:   20 | path dev: 0.06m | total:   968
   9° | actuator: 94% | error: 2.53° | -: 76% | =: 20% | +:  4% | lim:    6 | sat:    0 | path dev: 0.20m | total:    25
  10° | actuator: 99% | error: 3.52° | -:100% | =:  0% | +:  0% | lim:   10 | sat:   20 | path dev: 0.24m | total:    35
  11° | actuator:100% | error: 3.62° | -:100% | =:  0% | +:  0% | lim:    0 | sat:   21 | path dev: 0.31m | total:    32
  12° | actuator:100% | error: 3.71° | -:100% | =:  0% | +:  0% | lim:    0 | sat:   30 | path dev: 0.37m | total:    39
  13° | actuator:100% | error: 4.58° | -:100% | =:  0% | +:  0% | lim:    0 | sat:   38 | path dev: 0.37m | total:    52
  14° | actuator:100% | error: 5.41° | -:100% | =:  0% | +:  0% | lim:    0 | sat:   78 | path dev: 0.39m | total:    78

speed group: medium                                                       15 - 25 m/s  //  54 - 90 km/h  //  34 - 56 mph
  ----------------------------------------------------------------------------------------------------------------------
   0° | actuator: 18% | error: 0.41° | -: 61% | =:  7% | +: 31% | lim: 1300 | sat:    0 | path dev: 0.04m | total: 10043
   1° | actuator: 26% | error: 0.68° | -: 87% | =:  3% | +:  8% | lim: 1483 | sat:    0 | path dev: 0.05m | total: 11562
   2° | actuator: 39% | error: 1.03° | -: 94% | =:  0% | +:  4% | lim:  684 | sat:    0 | path dev: 0.06m | total:  5150
   3° | actuator: 41% | error: 1.01° | -: 73% | =:  3% | +: 22% | lim:  153 | sat:    0 | path dev: 0.08m | total:   931
   4° | actuator: 50% | error: 0.85° | -: 61% | =:  0% | +: 37% | lim:  112 | sat:    0 | path dev: 0.04m | total:   530
   5° | actuator: 54% | error: 0.52° | -: 43% | =:  6% | +: 50% | lim:   73 | sat:    0 | path dev: 0.03m | total:   427
   6° | actuator: 62% | error: 0.48° | -: 48% | =:  3% | +: 47% | lim:  103 | sat:    0 | path dev: 0.04m | total:   849
   7° | actuator: 70% | error: 0.70° | -: 53% | =:  0% | +: 46% | lim:   48 | sat:    0 | path dev: 0.04m | total:   380
   8° | actuator: 93% | error: 1.13° | -: 93% | =:  0% | +:  6% | lim:   14 | sat:    0 | path dev: 0.09m | total:    80

He's having pingpong problems on the straights. You can see in the results for the 1 and 2 degree angles he not only has relatively high error, but the error is very heavily weighted in one direction. This may be due to road crown, or perhaps front-end alignment. I think his EPS doesn't do any straight-line driving compensation, or perhaps it isn't factored into the LKAS API that openpilot uses.

This is surprisingly un-obvious in PlogJuggler unless you're really looking for it, but the accuracy script from this PR highlights it nicely. If the other Acadias out there have similar problems, they probably need some integral. Acadia integral is set to zero right now in upstream master.

@pd0wm
Copy link
Contributor

pd0wm commented May 12, 2022

I'll just merge this, we can always make more improvements later.

@pd0wm pd0wm merged commit c007c7e into commaai:master May 12, 2022
@jyoung8607 jyoung8607 deleted the steering-stats-fix branch May 12, 2022 13:43
spektor56 pushed a commit to spektor56/ghostpilot that referenced this pull request May 14, 2022
* move steering accuracy measurement script

* git rebase is utterly worthless

* fix header width
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants