autowarefoundation / autoware.universe

https://autowarefoundation.github.io/autoware.universe/
Apache License 2.0
953 stars 621 forks source link

The LiDAR azimuth unit is ambiguous #1127

Closed badai-nguyen closed 1 year ago

badai-nguyen commented 2 years ago

Checklist

Description

Currently, the LiDAR's azimuth is output in hundredths of degree unit that makes the code unclear and error-prone when adding support for new LiDAR devices. This problem was recognized when adding support for a new LiDAR in this issue: #509

Purpose

Improve the code readability and ease of adding support for new LiDARs.

Possible approaches

The degree unit seems easier for readers to imagine the value of angle setting, while the radian unit may give performance benefits for conversions in sinusoidal/co-sinusoidal related function. Since the main purpose is for readability, it is proposed to use the unit of degree for azimuth.

Definition of done

Unify the new unit of azimuth in AW.

stale[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity.

xmfcx commented 1 year ago

My suggestion: Following the discussions for the new point type PointXYZIRCT I think we can do the following:

@aohsato @miursh

Screenshot from 2022-11-15 16-52-13

VLP-16 User Manual

miursh commented 1 year ago

@xmfcx In my opinion, calculating the azimuth for each point at the place requires extra calculation cost even though the drivers don't have to calculate since they know the exact azimuth based on the hardware laser configurations or based on sensor output. Since we handle more than 200,000 points (e.g. for VLS128), I think we should avoid those extra computational cost

xmfcx commented 1 year ago

@xmfcx check for how long it takes to compute std::hypot and std::atan2 for large amount of points just to see how much computation it takes.

Also list what nodes make use of azimuth and distance fields.

@esteve also suggests to send down the values because storage/memory is cheaper.

xmfcx commented 1 year ago

I've created a mini benchmark for this topic: https://github.com/xmfcx/bench_point_cloud#readme @esteve @miursh

Let's continue this discussion in https://github.com/autowarefoundation/autoware-documentation/pull/180#issuecomment-1317583569