feat(autoware_lanelet2_map_validator): introduce autoware_lanelet2_map_validator #118

TaikiYamada4 · 2024-09-13T02:06:28Z

Description

This PR adds a new tool autoware_lanelet2_map_validator.
autoware_lanelet2_map_validator validates Lanelet2 maps whether it can work with Autoware.

Tests performed

There are two ways to test this tool.

1. colcon test

Run the following command for colcon test, (--event-handlers console_cohesion+ is optional).
This test should pass 100%.

colcon test --packages-select autoware_lanelet2_map_validator --event-handlers console_cohesion+

2. Test support commands

Check that these two commands work

ros2 run autoware_lanelet2_map_validator autoware_lanelet2_map_validator --help
ros2 run autoware_lanelet2_map_validator autoware_lanelet2_map_validator --print

The print option output like,

The following checks are available:
mapping.bool_tags
mapping.crosswalk.missing_regulatory_elements
mapping.crosswalk.regulatory_element_details
mapping.curvature_too_big
mapping.duplicated_points
mapping.mandatory_tags
mapping.points_too_close
mapping.stop_line.missing_regulatory_elements
mapping.traffic_light.missing_regulatory_elements
mapping.traffic_light.regulatory_element_details
mapping.unknown_tag_value
mapping.unknown_tags
mapping.validator.template
routing.graph_is_valid

3. Run the validator to an actual map with a single validator

You can run this tool with the following command.

ros2 run autoware_lanelet2_map_validator autoware_lanelet2_map_validator -m "target_lanelet2_map.osm" -v "validator.name"

validator.name can be selected from the list you obtained from the --print command.

This is an example of validating the map from the planning_simulator tutorial.

ros2 run autoware_lanelet2_map_validator autoware_lanelet2_map_validator -m ~/autoware_map/sample-map-planning/lanelet2_map.osm -v mapping.crosswalk.regulatory_element_details

Then, you might get

0 issues found.

4. Run the validator to an actual map with a requirement set json file

You can run this tool with multiple validators at once. For example, this command will validate "target_lanelet2_map.osm" with the validators listed in autoware_requirement_set.json, and output the results to the current directory.

ros2 run autoware_lanelet2_map_validator autoware_lanelet2_map_validator -m "target_lanelet2_map.osm" -i autoware_requirement_set.json -o ./

If "target_lanelet2_map.osm" is the one from the planning_simulator tutorial, you might get this

[vm-04-01] Failed
  - mapping.crosswalk.missing_regulatory_elements: Failed
  - mapping.crosswalk.regulatory_element_details: Passed
Error: lanelet 163 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 164 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 165 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 166 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
4 issues found.

[vm-05-01] Failed
  - mapping.traffic_light.missing_regulatory_elements: Failed
  - mapping.traffic_light.regulatory_element_details: Passed
Error: linestring 9776 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 9774 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 9771 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 9769 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 340 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 342 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 345 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
Error: linestring 347 No regulatory element refers to this traffic light. [mapping.traffic_light.missing_regulatory_elements]
8 issues found.

Total of 12 errors were found from /home/taikiyamada/autoware_map/sample-map-planning/lanelet2_map.osm
Results are output to .//lanelet2_validation_results.json
[ros2run]: Process exited with failure 1

If "target_lanelet2_map.osm" is the autoware.universe/common/autoware_test_utils/test_map/lanelet2_map.osm, you might get this

[vm-04-01] Failed
  - mapping.crosswalk.missing_regulatory_elements: Failed
  - mapping.crosswalk.regulatory_element_details: Passed
Error: lanelet 163 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 164 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 165 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
Error: lanelet 166 No regulatory element refers to this crosswalk. [mapping.crosswalk.missing_regulatory_elements]
4 issues found.

[vm-05-01] Failed
  - mapping.traffic_light.missing_regulatory_elements: Passed
  - mapping.traffic_light.regulatory_element_details: Failed
Error: regulatory element 9896 Regulatory element of traffic light must have a stop line(ref_line). [mapping.traffic_light.regulatory_element_details]
Error: regulatory element 9918 Regulatory element of traffic light must have a stop line(ref_line). [mapping.traffic_light.regulatory_element_details]
Error: regulatory element 9838 Regulatory element of traffic light must have a stop line(ref_line). [mapping.traffic_light.regulatory_element_details]
Error: regulatory element 9874 Regulatory element of traffic light must have a stop line(ref_line). [mapping.traffic_light.regulatory_element_details]
4 issues found.

Total of 8 errors were found from ./src/universe/autoware.universe/common/autoware_test_utils/test_map/lanelet2_map.osm
Results are output to .//lanelet2_validation_results.json
[ros2run]: Process exited with failure 1

Notes for reviewers

Read the README.md for further information of this tool.

Effects on system behavior

None, because this tool doesn't work with Autoware together.

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: TaikiYamada4 <[email protected]>

Removed rclcpp which is unused. Signed-off-by: TaikiYamada4 <[email protected]>

Added validation template Signed-off-by: TaikiYamada4 <[email protected]>

Added yaml input/output for a set of validations Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

Fixed mistakes in README.md Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

Fixed pre-commit.ci related errors. Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

map/autoware_lanelet2_map_validator/src/lib/cli.cpp

Refactor code style Signed-off-by: TaikiYamada4 <[email protected]>

map/autoware_lanelet2_map_validator/src/lib/validation.cpp

Delete unused variables Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 · 2024-10-25T10:05:08Z

I believe this improved the error handling
d16b54b

And this improved the style of desc.add_options() and deleted unused checks and parse_filter
8b93027

...anelet2_map_validator/src/validators/crosswalk/missing_regulatory_elements_for_crosswalk.cpp

Restrict input/output format. Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

codecov · 2024-10-28T07:09:24Z

Codecov Report

Attention: Patch coverage is 27.72277% with 219 lines in your changes missing coverage. Please review.

Project coverage is 27.72%. Comparing base (ad0b811) to head (b405d14).
Report is 58 commits behind head on main.

Files with missing lines	Patch %	Lines
map/autoware_lanelet2_map_validator/src/main.cpp	0.00%	66 Missing ⚠️
...ap/autoware_lanelet2_map_validator/src/lib/cli.cpp	0.00%	45 Missing ⚠️
...ware_lanelet2_map_validator/src/lib/validation.cpp	0.00%	24 Missing ⚠️
...walk/regulatory_element_details_for_crosswalks.cpp	40.00%	8 Missing and 16 partials ⚠️
.../regulatory_element_details_for_traffic_lights.cpp	46.42%	2 Missing and 13 partials ⚠️
...walk/missing_regulatory_elements_for_crosswalk.cpp	46.15%	2 Missing and 12 partials ⚠️
...ine/missing_regulatory_elements_for_stop_lines.cpp	62.50%	2 Missing and 7 partials ⚠️
...missing_regulatory_elements_for_traffic_lights.cpp	64.00%	2 Missing and 7 partials ⚠️
.../autoware_lanelet2_map_validator/src/lib/utils.hpp	70.58%	0 Missing and 5 partials ⚠️
...ap_validator/src/validators/validator_template.cpp	0.00%	2 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff             @@
##            main     #118       +/-   ##
==========================================
+ Coverage   2.12%   27.72%   +25.59%     
==========================================
  Files        163       16      -147     
  Lines       9402      303     -9099     
  Branches     383      193      -190     
==========================================
- Hits         200       84      -116     
+ Misses      9045      154     -8891     
+ Partials     157       65       -92

Flag	Coverage Δ
differential	`27.72% <27.72%> (?)`
total	`?`

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

map/autoware_lanelet2_map_validator/README.md

map/autoware_lanelet2_map_validator/docs/crosswalk/regulatory_element_details_for_crosswalks.md

Fixed explanation of issues Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

wenrir

Minor comments on this PR, might be completely irrelevant, in that case sorry.
Thank you for your consideration.

wenrir · 2024-10-28T08:38:45Z

map/autoware_lanelet2_map_validator/src/lib/validation.cpp

+  } else {
+    // std::cerr << "Set to default projector: MGRS projector" << std::endl;
+    return std::make_unique<lanelet::projection::MGRSProjector>();
+  }


Is this branch necessary?

Suggested change

} else {

// std::cerr << "Set to default projector: MGRS projector" << std::endl;

return std::make_unique<lanelet::projection::MGRSProjector>();

}

return std::make_unique<lanelet::projection::MGRSProjector>();

wenrir · 2024-10-28T08:39:34Z

map/autoware_lanelet2_map_validator/src/lib/validation.hpp

@@ -0,0 +1,52 @@
+// Copyright 2023 Autoware Foundation


Suggested change

// Copyright 2023 Autoware Foundation

// Copyright 2024 Autoware Foundation

wenrir · 2024-10-28T08:40:16Z

map/autoware_lanelet2_map_validator/src/lib/cli.cpp

@@ -0,0 +1,107 @@
+// Copyright 2023 Autoware Foundation


Suggested change

// Copyright 2023 Autoware Foundation

// Copyright 2024 Autoware Foundation

Maybe this should be up-to-date?

wenrir · 2024-10-28T08:40:26Z

map/autoware_lanelet2_map_validator/src/lib/cli.hpp

@@ -0,0 +1,45 @@
+// Copyright 2023 Autoware Foundation


Suggested change

// Copyright 2023 Autoware Foundation

// Copyright 2024 Autoware Foundation

wenrir · 2024-10-28T08:40:39Z

map/autoware_lanelet2_map_validator/src/lib/utils.hpp

@@ -0,0 +1,85 @@
+// Copyright 2023 Autoware Foundation


Suggested change

// Copyright 2023 Autoware Foundation

// Copyright 2024 Autoware Foundation

Maybe this should be up to date

wenrir · 2024-10-28T08:42:18Z

map/autoware_lanelet2_map_validator/src/lib/utils.hpp

+      if (new_it != in_vec.end()) {
+        iter = new_it;
+      } else {
+        break;
+      }


Is the else necessary? as this will break regardless

Suggested change

if (new_it != in_vec.end()) {

iter = new_it;

} else {

break;

}

if (new_it != in_vec.end()) {

iter = new_it;

}

break;

wenrir · 2024-10-28T08:47:15Z

map/autoware_lanelet2_map_validator/src/lib/validation.cpp

+    const auto & projector = getProjector(config);
+    map = lanelet::load(cm_config.mapFile, *projector, &errors);
+    if (!errors.empty()) {
+      std::cout << "!errors.empty()" << std::endl;


Suggested change

std::cout << "!errors.empty()" << std::endl;

Seems not relevant outside of debugging?

wenrir · 2024-10-28T09:00:31Z

map/autoware_lanelet2_map_validator/src/lib/utils.hpp

+}
+
+template <typename T>
+void checkPrimitivesType(


[suggestion] could possibly introduce pack expansion for the template template<typename T, typename ... Ts> to handle expected type / subtype and in that way reduce the duplication.

map/autoware_lanelet2_map_validator/test/src/test_missing_regulatory_elements.cpp

Refined CMakeLists.txt Signed-off-by: TaikiYamada4 <[email protected]>

Signed-off-by: TaikiYamada4 <[email protected]>

Removed redundant else statement. Removed debug comments Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 · 2024-10-29T09:12:48Z

@YamatoAndo @wenrir
Thank you for your comments! I have revised my code as you two mentioned except one thing.
074c4f2

@wenrir
I understand that creating a second template parameter could reduce duplication by consolidating the two function definitions. However, since the only extension here involves adding a subtype specifically for searching, I prefer to use function overloading to keep the second function definition for clarity.

map/autoware_lanelet2_map_validator/test/src/test_regulatory_element_details.cpp

Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 added 2 commits September 13, 2024 09:55

introduce autoware_lanelet2_map_validator to autoware_tools

1d9f97e

Signed-off-by: TaikiYamada4 <[email protected]>

wrote description a little to README.md

43d2441

Signed-off-by: TaikiYamada4 <[email protected]>

github-actions bot added type:documentation component:map labels Sep 13, 2024

pre-commit-ci bot and others added 16 commits September 13, 2024 02:06

style(pre-commit): autofix

0419b74

Restore commented out parts.

37daad6

Removed rclcpp which is unused. Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

b6d8b36

Separate validation rules to samller pieces.

77416f2

Added validation template Signed-off-by: TaikiYamada4 <[email protected]>

Split the validation code into smaller pieces.

ebefd7c

Added yaml input/output for a set of validations Signed-off-by: TaikiYamada4 <[email protected]>

Fixed test codes to use the separated codes

ea38dd1

Signed-off-by: TaikiYamada4 <[email protected]>

Removed unused code which are already divided to smaller codes.

e091ff6

Signed-off-by: TaikiYamada4 <[email protected]>

Rename new_main.cpp to main.cpp

359f359

Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

e622c91

Wrote detailed README.md

37489ec

Signed-off-by: TaikiYamada4 <[email protected]>

Fixed commit mistake

b653c84

Signed-off-by: TaikiYamada4 <[email protected]>

Renew input command option to -i from -r.

27b354d

Fixed mistakes in README.md Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

9014a3c

Fixed long to uint64_t

fcbc7d8

Signed-off-by: TaikiYamada4 <[email protected]>

Fixed spelling

1685e7e

Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

077e7e6

TaikiYamada4 self-assigned this Sep 30, 2024

TaikiYamada4 and others added 6 commits September 30, 2024 14:28

Fixed typo

18524be

Signed-off-by: TaikiYamada4 <[email protected]>

Split long lines in the code

dfdb4f0

Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

ef09205

Changed the entire structure.

4e42996

Fixed pre-commit.ci related errors. Signed-off-by: TaikiYamada4 <[email protected]>

style(pre-commit): autofix

f9a16da

Fixed pre-commit.ci related stuff

147ba6a

Signed-off-by: TaikiYamada4 <[email protected]>

TaikiYamada4 requested review from RyuYamamoto, anhnv3991 and YamatoAndo September 30, 2024 09:46

YamatoAndo reviewed Oct 25, 2024

View reviewed changes

map/autoware_lanelet2_map_validator/src/lib/cli.cpp Outdated Show resolved Hide resolved

Improve error handling

d16b54b

Refactor code style Signed-off-by: TaikiYamada4 <[email protected]>

YamatoAndo reviewed Oct 25, 2024

View reviewed changes

map/autoware_lanelet2_map_validator/src/lib/validation.cpp Outdated Show resolved Hide resolved

Avoid clang format

8b93027

Delete unused variables Signed-off-by: TaikiYamada4 <[email protected]>

YamatoAndo reviewed Oct 25, 2024

View reviewed changes

...anelet2_map_validator/src/validators/crosswalk/missing_regulatory_elements_for_crosswalk.cpp Outdated Show resolved Hide resolved

YamatoAndo reviewed Oct 25, 2024

View reviewed changes

...anelet2_map_validator/src/validators/crosswalk/missing_regulatory_elements_for_crosswalk.cpp Outdated Show resolved Hide resolved

TaikiYamada4 added 4 commits October 28, 2024 10:40

Removed redundant process.

c2a437c

Restrict input/output format. Signed-off-by: TaikiYamada4 <[email protected]>

Added approaches to the documents

6f47702

Signed-off-by: TaikiYamada4 <[email protected]>

Fixed typo

c114991

Signed-off-by: TaikiYamada4 <[email protected]>

Removed catch and improve io error handling

fe9d19a

Signed-off-by: TaikiYamada4 <[email protected]>