fix(ndt_scan_matcher): using de-ground points calculate ndt match score #2312

cyn-liu · 2022-11-17T11:08:36Z

Signed-off-by: Cynthia Liu cynthia.liu@autocore.ai

Description

related issue: 2044.
As discussed in issue 2044, using de-grounded point cloud in NDT matching for localization may achieve better results, so I have made some changes to the code:

NDT matched positioning using the original point cloud(/localization/util/downsample/pointcloud).
Calculates NDT match scores using de-grounded point cloud(/localization/util_no_ground/downsample/pointcloud).

In the ndt_scan_matcher node I subscribe to both topics in a callback function, one topic is used to locate and the other is used to calculate the scores(transform_probability & nearest_voxel_transformation_likelihood).

Tests performed

Test done.
Test results refer to the previous issue link.

Notes for reviewers

Pre-review checklist for the PR author

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

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.
The PR has been properly tested.
The PR has been reviewed by the code owners.

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.
The PR is ready for merge.

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

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

KYabuuchi

Thank you for your cooperation!
I found a mistake that causes localization_launch to fail. Please check it.

launch/tier4_localization_launch/launch/localization.launch.xml

kminoda

Thank you very much for your contribution!! I really appreciate this proposal.
Basically, I agree that the score with ground removal is worth considering as an alternative scan matching performance indicator.
I left some general comments.

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp

codecov · 2022-11-18T07:47:33Z

Codecov Report

Base: 11.27% // Head: 11.25% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (508ba97) compared to base (c958fdd).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2312      +/-   ##
==========================================
- Coverage   11.27%   11.25%   -0.03%     
==========================================
  Files        1168     1168              
  Lines       81943    82096     +153     
  Branches    21012    21012              
==========================================
  Hits         9242     9242              
- Misses      62838    62991     +153     
  Partials     9863     9863

Flag	Coverage Δ		*Carryforward flag
differential	`0.00% <0.00%> (?)`
total	`11.27% <0.00%> (ø)`		Carriedforward from c958fdd

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

Impacted Files	Coverage Δ
...ion/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp	`0.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

KYabuuchi

It looks like the modification was complete, so I added some comments.
Please check them. 😄

launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml

localization/ndt_scan_matcher/include/ndt_scan_matcher/ndt_scan_matcher_core.hpp

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp

launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml

YamatoAndo · 2022-11-29T09:45:40Z

@cyn-liu Thank you for the update!
I have one question I would like to discuss.
If the ground is not included in the LiDAR point cloud, there is a risk that RANSAC will detect building walls, etc.
So, I think it would be better to simply filter by z-coordinate values.
What do you think?

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

kminoda · 2022-12-07T06:42:30Z

@cyn-liu I think it's due to this commit: ce5e633
I manually set DCO CI to pass for now.

xmfcx · 2022-12-13T15:27:09Z

@kminoda Is this ready to be merged? If so can you approve it?

kminoda

Sorry for being late. LGTM

mitsudome-r

Please update the README for the added parameters. https://github.com/autowarefoundation/autoware.universe/tree/5d5f091f035ac1efce07baed6a9fd80ab7b876f9/localization/ndt_scan_matcher

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu · 2022-12-21T02:34:19Z

I modify the README.md in ndt_scan_matcher package.

kminoda · 2022-12-21T09:55:01Z

@cyn-liu Since the parameter management configuration has been changed, please modify launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml
instead of launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml.

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu · 2022-12-23T05:45:33Z

@kminoda Modified.

liuXinGangChina · 2023-01-09T03:27:39Z

Hi,Mitsudome-san @mitsudome-r . Since Yamato-san and kminoda-san have approved this pr, will you help us to merge it?

Best regards,
Lucas

cyn-liu · 2023-01-09T03:55:30Z

@cyn-liu Since the parameter management configuration has been changed, please modify launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml instead of launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml.

Hello @kminoda , I have a question:
The file launch/tier4_localization_launch/config/ndt_scan_matcher.param.yaml has been transferred to autoware_launch/config/localization/ndt_scan_matcher.param.yaml , and I have updated this change in autoware.universe repo, If I want to add my parameters in autoware_launch repo, should I need create a new PR in this repo?

kminoda · 2023-01-10T00:17:44Z

@cyn-liu Yes, your understanding is correct!
(We are also considering adding a workflow based on GitHub Actions that syncs parameters in autoware.universe and autoware_launch, but for now, please update both parameters manually 🙏 )

kminoda · 2023-01-10T04:34:50Z

@liuXinGangChina It seems that one CI (DCO) failed, and it kept this PR from being able to merge.
Would you check it out again? I think now this PR can be merged (I set the DCO to pass)

kminoda · 2023-01-11T00:21:57Z

I've also merged this PR autowarefoundation/autoware_launch#155

…re (autowarefoundation#2312) Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu added 3 commits November 17, 2022 11:15

add remote

ce5e633

Merge remote-tracking branch 'remotes/origin/main' into main

b2c2978

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

3f99083

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu requested review from KYabuuchi, kminoda and YamatoAndo as code owners November 17, 2022 11:08

github-actions bot added component:launch Launch files, scripts and initialization tools. (auto-assigned) component:localization Vehicle's position determination in its environment. (auto-assigned) labels Nov 17, 2022

cyn-liu changed the title ~~fix(ndt_scan_matcher): Use de-ground points calculate ndt match score~~ fix(ndt_scan_matcher): using de-ground points calculate ndt match score Nov 17, 2022

cyn-liu force-pushed the use-deground-points-calculate-ndt-match-score branch from 24b362a to 3f99083 Compare November 17, 2022 11:31

ci(pre-commit): autofix

d3da59a

KYabuuchi reviewed Nov 18, 2022

View reviewed changes

launch/tier4_localization_launch/launch/localization.launch.xml Outdated Show resolved Hide resolved

kminoda reviewed Nov 18, 2022

View reviewed changes

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp Outdated Show resolved Hide resolved

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp Outdated Show resolved Hide resolved

kminoda assigned kminoda and cyn-liu and unassigned kminoda Nov 18, 2022

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

3191d9f

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu requested a review from a team as a code owner November 21, 2022 04:25

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

4da0842

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

kminoda reviewed Nov 21, 2022

View reviewed changes

localization/ndt_scan_matcher/src/ndt_scan_matcher_core.cpp Outdated Show resolved Hide resolved

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

99c7621

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu force-pushed the use-deground-points-calculate-ndt-match-score branch from ef6d148 to 99c7621 Compare November 28, 2022 08:16

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

f1bf194

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu force-pushed the use-deground-points-calculate-ndt-match-score branch from d4ac1bb to f1bf194 Compare November 28, 2022 08:24

KYabuuchi reviewed Nov 29, 2022

View reviewed changes

kminoda reviewed Nov 29, 2022

View reviewed changes

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

378e417

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu force-pushed the use-deground-points-calculate-ndt-match-score branch from 0e9ccbd to 378e417 Compare November 29, 2022 10:21

xmfcx added this to the Bus ODD Nov-Dec Milestone milestone Dec 13, 2022

kminoda approved these changes Dec 14, 2022

View reviewed changes

YamatoAndo approved these changes Dec 14, 2022

View reviewed changes

mitsudome-r reviewed Dec 20, 2022

View reviewed changes

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

f5301cb

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

cyn-liu force-pushed the use-deground-points-calculate-ndt-match-score branch from 5d5f091 to f5301cb Compare December 21, 2022 02:29

github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Dec 21, 2022

ci(pre-commit): autofix

e9502a2

cyn-liu and others added 3 commits December 22, 2022 15:19

Merge branch 'autowarefoundation:main' into main

a16da44

Merge branch 'autowarefoundation:main' into main

3940b8f

fix(ndt_scan_matcher): using de-ground points calculate ndt match score

508ba97

Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

github-actions bot removed the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Dec 23, 2022

BonoloAWF linked an issue Jan 6, 2023 that may be closed by this pull request

Using a de-grounded point cloud for NDT matching gives a more reasonable match score #2044

Closed

3 tasks

BonoloAWF modified the milestones: Bus ODD Nov-Dec Milestone, Bus ODD Jan-Feb Milestone Jan 6, 2023

cyn-liu mentioned this pull request Jan 10, 2023

fix(autoware-launch): add NDT parameters for de-grounded lidar scan matching autowarefoundation/autoware_launch#155

Merged

7 tasks

xmfcx merged commit bdeeb97 into autowarefoundation:main Jan 10, 2023

tier4-autoware-public-bot bot mentioned this pull request Jan 11, 2023

chore: sync upstream tier4/autoware.universe#221

Merged

badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Mar 22, 2023

fix(ndt_scan_matcher): using de-ground points calculate ndt match sco…

2f85a20

…re (autowarefoundation#2312) Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Apr 4, 2023

fix(ndt_scan_matcher): using de-ground points calculate ndt match sco…

d261aea

…re (autowarefoundation#2312) Signed-off-by: Cynthia Liu <cynthia.liu@autocore.ai>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ndt_scan_matcher): using de-ground points calculate ndt match score #2312

fix(ndt_scan_matcher): using de-ground points calculate ndt match score #2312

cyn-liu commented Nov 17, 2022 •

edited

Loading

KYabuuchi left a comment

kminoda left a comment •

edited

Loading

codecov bot commented Nov 18, 2022 •

edited

Loading

KYabuuchi left a comment

YamatoAndo commented Nov 29, 2022

kminoda commented Dec 7, 2022

xmfcx commented Dec 13, 2022

kminoda left a comment

mitsudome-r left a comment

cyn-liu commented Dec 21, 2022

kminoda commented Dec 21, 2022

cyn-liu commented Dec 23, 2022

liuXinGangChina commented Jan 9, 2023 •

edited

Loading

cyn-liu commented Jan 9, 2023

kminoda commented Jan 10, 2023

kminoda commented Jan 10, 2023

kminoda commented Jan 11, 2023

fix(ndt_scan_matcher): using de-ground points calculate ndt match score #2312

fix(ndt_scan_matcher): using de-ground points calculate ndt match score #2312

Conversation

cyn-liu commented Nov 17, 2022 • edited Loading

Description

Related links

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

KYabuuchi left a comment

Choose a reason for hiding this comment

kminoda left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Nov 18, 2022 • edited Loading

Codecov Report

KYabuuchi left a comment

Choose a reason for hiding this comment

YamatoAndo commented Nov 29, 2022

kminoda commented Dec 7, 2022

xmfcx commented Dec 13, 2022

kminoda left a comment

Choose a reason for hiding this comment

mitsudome-r left a comment

Choose a reason for hiding this comment

cyn-liu commented Dec 21, 2022

kminoda commented Dec 21, 2022

cyn-liu commented Dec 23, 2022

liuXinGangChina commented Jan 9, 2023 • edited Loading

cyn-liu commented Jan 9, 2023

kminoda commented Jan 10, 2023

kminoda commented Jan 10, 2023

kminoda commented Jan 11, 2023

cyn-liu commented Nov 17, 2022 •

edited

Loading

kminoda left a comment •

edited

Loading

codecov bot commented Nov 18, 2022 •

edited

Loading

liuXinGangChina commented Jan 9, 2023 •

edited

Loading