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

feat(map): add differential lanelet loading #6241

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

StepTurtle
Copy link

@StepTurtle StepTurtle commented Jan 30, 2024

Description

Dynamically load Lanelet2 maps required for the current driving situation, aiming to avoid time consumption and reduce computational costs in the planning pipeline, while also addressing long-term issues associated with Lanelet2 map projection.

Related links

Proposal Link

Tests performed

You can check the update comment in proposal to see current test results.

short.mp4

To test on your local environment, follow these steps:

  1. Download the example map from the following link: link
  2. Ensure you have the latest changes related to dynamic lanelet loading by incorporating the following PRs:
  1. Make true enable_differential_map_loading argument from src/launch/autoware_launch/autoware_launch/launch/components/tier4_planning_component.launch file:
  2. Make true enable_differential_map_loading argument from src/launch/autoware_launch/autoware_launch/launch/components/tier4_map_component.launch file:
  3. Run planning simulator and add /map/dynamic_vector_map_marker topic to Rviz. After you provide a 2D position, the dynamic should be loaded. (It looks with dark grey color.)
  • ros2 launch autoware_launch planning_simulator.launch.xml map_path:=$HOME/autoware_map/dummy_800_4/ vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit

Documentation:

Notes for reviewers

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

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

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.

@github-actions github-actions bot added the component:map Map creation, storage, and loading. (auto-assigned) label Jan 30, 2024
@github-actions github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Feb 9, 2024
@StepTurtle StepTurtle changed the title feat(map_loader): add differential lanelet loading feat(map): add differential lanelet loading Feb 9, 2024
@github-actions github-actions bot added the component:common Common packages from the autoware-common repository. (auto-assigned) label Feb 12, 2024
@mitsudome-r mitsudome-r marked this pull request as ready for review February 19, 2024 08:30
@mitsudome-r
Copy link
Member

@StepTurtle Thanks for the PR.
Did you have any chance to test with this PR to see if nodes that receives lanelet map work correctly or not? For example, I would like to make sure that there is no rise in latency with planning output when the planner nodes receives a new map tile since they would have to do preprocessing of the map before it can be used for planning.

@StepTurtle StepTurtle force-pushed the feat/add_dynamic_lanelet_loading branch from 30523b6 to beb7faf Compare September 3, 2024 11:51
Signed-off-by: Barış Zeren <[email protected]>
@YamatoAndo
Copy link
Contributor

@StepTurtle Hi,
If you have some time, I would appreciate it if you could use linters (cpplint, cppcheck, clang-tidy) to improve the code, referencing #7845. However, since this task can take a considerable amount of time, we can also ask TIER IV members to handle it after merging this PR #6241. What do you think?

@StepTurtle
Copy link
Author

@StepTurtle Hi, If you have some time, I would appreciate it if you could use linters (cpplint, cppcheck, clang-tidy) to improve the code, referencing #7845. However, since this task can take a considerable amount of time, we can also ask TIER IV members to handle it after merging this PR #6241. What do you think?

Hi @YamatoAndo

From my perspective, it's been a bit difficult to dedicate time to this task as I've been working on other tasks for a while. Therefore, if we can find someone else who can take care of this task, it would be better for me.

However, if we have concerns about merging without these tests and checks being done, I will try to find time to work on it. Please inform me.

@YamatoAndo
Copy link
Contributor

@StepTurtle

From my perspective, it's been a bit difficult to dedicate time to this task as I've been working on other tasks for a while. Therefore, if we can find someone else who can take care of this task, it would be better for me.

OK! I would like to ask the TIER IV members to take on this task after merging.

@YamatoAndo
Copy link
Contributor

@StepTurtle
I think the source code is good as it is, but there are three more things I'd like you to do

  1. I would like the lanelet2-map-tile-generator to be placed in a location that Autoware users can easily find. I think it would be best to move this tool to the autoware_tools repository. Alternatively, if you prefer to manage it as part of the leo-drive repository, adding it to the tools.repos file would also be a good option.

  2. I would like you to add an explanation of the metadata.yaml structure and introduce the lanelet2-map-tile-generator in the README for the map_loader. If you don't have time, please let me know, and after this PR is merged, I can ask the TIER IV members to update the README.

  3. If you don't mind, in order to maintain the autoware_dynamic_lanelet_provider moving forward, I would like to add TIER IV members as maintainers.

  <maintainer email="[email protected]">Yamato Ando</maintainer>
  <maintainer email="[email protected]">Ryu Yamamoto</maintainer>
  <maintainer email="[email protected]">Masahiro Sakamoto</maintainer>
  <maintainer email="[email protected]">NGUYEN Viet Anh</maintainer>
  <maintainer email="[email protected]">Taiki Yamada</maintainer>
  <maintainer email="[email protected]">Shintaro Sakoda</maintainer>

@StepTurtle
Copy link
Author

StepTurtle commented Sep 5, 2024

@StepTurtle I think the source code is good as it is, but there are three more things I'd like you to do

Hi @YamatoAndo, you mentioned on good points, thanks.

1) The most likely best method would be to unify it with point_cloud_divider. Therefore, I support the idea of placing it inside autoware_tools. However, for this, we will first need to convert the tool into a ROS 2 package, and I believe we can get this done in a short time.

My concern with this is that the package has some third-party depends (osmium-tool, gdal). If we expect autoware_tools to be correctly set up with all the libraries installed by the ./setup-dev-env.sh script, we will need to make additional installations for the lanelet-tile-generator that we will add. This means that anyone who wants to compile the autoware_tools repository will need to install these dependencies or use the --packages-skip flag when running the colcon build command. I am not sure how can we handle this. There might be something I missed here; please complete anything if there's something missing.

2) There were two thing which updates documentation. (These links can find at top of the PR)

Can you check both of them and inform me regarding whether the content is sufficient or not. (btw these two are very old, for example, they were prepared according to the old metadata.yaml and so on. So I will update as soon as possible if we OK on these docs)

Additionally, if we think there is a need to add a general document under the how-to-guides, I can take care of that.

3) Maintainers updated with TIER IV members.

related commit: 999e023

Signed-off-by: Barış Zeren <[email protected]>
@StepTurtle StepTurtle force-pushed the feat/add_dynamic_lanelet_loading branch from 392f66c to 999e023 Compare September 5, 2024 09:05
@YamatoAndo
Copy link
Contributor

@StepTurtle Hi,
1.

My concern with this is that the package has some third-party depends (osmium-tool, gdal). If we expect autoware_tools to be correctly set up with all the libraries installed by the ./setup-dev-env.sh script, we will need to make additional installations for the lanelet-tile-generator that we will add. This means that anyone who wants to compile the autoware_tools repository will need to install these dependencies

It would be ideal if everything is set up correctly, but in autoware.tools, we allow for individual installation of third-party dependencies, as is done with bag2lanelet.

or use the --packages-skip flag when running the colcon build command.

Using the --packages-skip flag is not acceptable. In the first place, since lanelet2-map-tile-generator is composed solely of python scripts, it doesn't really require building anything, much like bag2lanelet. Therefore, I don't think the --packages-skip flag is necessary.

  1. Thank you! I believe there will be no issue as long as the content is updated to the latest version.

  2. Thank you so much!

@StepTurtle
Copy link
Author

It would be ideal if everything is set up correctly, but in autoware.tools, we allow for individual installation of third-party dependencies, as is done with bag2lanelet.

I see. Than we start to port the lanelet2-map-tile-generator to autoware_tools repository with @ataparlar . Should we unify the tool name with the point cloud divider, e.g. autoware_lanelet2_divider or do you prefer autoware_lanelet2_tile_generator

Also I will update the docs. ASAP.

@ataparlar
Copy link
Contributor

Hi @YamatoAndo

I am working on #7454 right now. I will implement the lanelet2-map-tile-generator code into the autoware_tools as soon as possible.

@StepTurtle
Copy link
Author

@YamatoAndo

Commit which updates map_loader README.md: 8308c18

PR which updates map component from autoware-documentation: autowarefoundation/autoware-documentation#522

@YamatoAndo
Copy link
Contributor

@StepTurtle @ataparlar

Than we start to port the lanelet2-map-tile-generator to autoware_tools repository with @ataparlar .

Thank you so much!

@YamatoAndo
Copy link
Contributor

@StepTurtle

Should we unify the tool name with the point cloud divider, e.g. autoware_lanelet2_divider or do you prefer autoware_lanelet2_tile_generator

I'd like to unify the name, but I'm okay with either option.

@anhnv3991 Hi, we are planning to include the Lanelet2 version of the point cloud divider in autoware_tools. Do you have any thoughts on the name?

map/map_loader/README.md Outdated Show resolved Hide resolved
@anhnv3991
Copy link
Contributor

@YamatoAndo Err, autoware_lanelet2_divider sounds good.

@YamatoAndo
Copy link
Contributor

@StepTurtle Please name it autoware_lanelet2_divider.

@StepTurtle
Copy link
Author

Hey @YamatoAndo kindly ping

Is there anything we are waiting for to merge the PRs? I just saw a conflict that occurred on the planning side, and I think it would be good for us to quickly complete the task.

If we're waiting for the divider tool, I will suggest merging the PRs before completing it, because it looks like we won't have much time to allocate to working on the tool in the next few weeks.

Comment on lines +61 to +62
return min_x <= getX(point) && getX(point) <= max_x && min_y <= getY(point) &&
getY(point) <= max_y;
Copy link
Contributor

Choose a reason for hiding this comment

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

@StepTurtle Please check the code correctly

Suggested change
return min_x <= getX(point) && getX(point) <= max_x && min_y <= getY(point) &&
getY(point) <= max_y;
return min_x <= getX(point) && getX(point) >= max_x && min_y <= getY(point) &&
getY(point) >= max_y;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:common Common packages from the autoware-common repository. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) component:map Map creation, storage, and loading. (auto-assigned) tag:deploy-docs Mark for deploy-docs action generation. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants