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

Simplify deskewing #427

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Simplify deskewing #427

merged 1 commit into from
Jan 24, 2025

Conversation

benemer
Copy link
Member

@benemer benemer commented Jan 24, 2025

After the discussion with @niosus in #421, it turned out that we can make the deskewing a bit easier to understand by using the original transformation from the mid_pose_timestamp solution, but with a fixed reference timestamp of 1.0. The motivation behind setting it to 1.0 can be found in the PR reference above.

This is mainly a cosmetic change, but it is more readable because we deskew and represent the scan at the end of the scan duration (in contrast to the current main, where we deskew to the beginning of the scan and then represent it at the end pose).

I checked the first 1000 scans of Riverside01, and we obtain the same results, and it's minimally faster (single thread for proper runtime comparison):

Main:

────────────────────────── Results for MulranDataset Sequence Riverside01 ──────────────────────────

+---------------------------------+-------+-------+
|                          Metric | Value | Units |
+=================================+=======+=======+
|       Average Translation Error | 3.862 | %     |
|        Average Rotational Error | 0.006 | deg/m |
| Absolute Trajectory Error (ATE) | 5.061 | m     |
| Absolute Rotational Error (ARE) | 0.425 | rad   |
|               Average Frequency |   6   | Hz    |
|                 Average Runtime |  155  | ms    |
+---------------------------------+-------+-------+

This PR:

────────────────────────── Results for MulranDataset Sequence Riverside01 ──────────────────────────
+---------------------------------+-------+-------+
|                          Metric | Value | Units |
+=================================+=======+=======+
|       Average Translation Error | 3.862 | %     |
|        Average Rotational Error | 0.006 | deg/m |
| Absolute Trajectory Error (ATE) | 5.061 | m     |
| Absolute Rotational Error (ARE) | 0.425 | rad   |
|               Average Frequency |   7   | Hz    |
|                 Average Runtime |  143  | ms    |
+---------------------------------+-------+-------+

@tizianoGuadagnino tizianoGuadagnino merged commit 8ac8269 into main Jan 24, 2025
21 checks passed
@tizianoGuadagnino tizianoGuadagnino deleted the benedikt/simplify_deskewing branch January 24, 2025 14:22
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.

3 participants