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

Fix/fix eigen variable definition #1324

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

hakuturu583
Copy link
Collaborator

@hakuturu583 hakuturu583 commented Jul 22, 2024

Description

Fixes: #1302, #1312

Abstract

Fix the definition of eigen variable for calculation of ego vehicle pose.

Background

In #1302, it is reported that kinematic state is not updated.

I have discovered a bug where pull requests from forks cannot be merged due to the Release Action malfunctioning.
Since #1313 is very important and needs to be incorporated urgently, @hakuturu583 will create a similar pull request and carry out verification and merging.

This Pull request is not @hakuturu583 's work, it is @MertClk 's work.
Thank you very much for contribution.

References

Reference to fix:

In short: do not use the auto keywords with Eigen's expressions, unless you are 100% sure about what you are doing. In particular, do not use the auto keyword as a replacement for a Matrix<> type.

This behavior is presumably due to the fact that the Eigen calculation is delayed by the Expression Template, resulting in undefined values. However, a tier4 internal verification showed that this part should not have been a problem even if auto had been used.
One possibility is that the behavior may be CPU architecture dependent.
The cause is unknown, but since there is no particular disadvantage, this PR is merged.

https://github.com/tier4/sim_evaluation_tools/issues/342
#1313

Copy link

Checklist for reviewers ☑️

All references to "You" in the following text refer to the code reviewer.

  • Is this pull request written in a way that is easy to read from a third-party perspective?
  • Is there sufficient information (background, purpose, specification, algorithm description, list of disruptive changes, and migration guide) in the description of this pull request?
  • If this pull request contains a destructive change, does this pull request contain the migration guide?
  • Labels of this pull request are valid?
  • All unit tests/integration tests are included in this pull request? If you think adding test cases is unnecessary, please describe why and cross out this line.
  • The documentation for this pull request is enough? If you think adding documents for this pull request is unnecessary, please describe why and cross out this line.

Copy link

Checklist for reviewers ☑️

All references to "You" in the following text refer to the code reviewer.

  • Is this pull request written in a way that is easy to read from a third-party perspective?
  • Is there sufficient information (background, purpose, specification, algorithm description, list of disruptive changes, and migration guide) in the description of this pull request?
  • If this pull request contains a destructive change, does this pull request contain the migration guide?
  • Labels of this pull request are valid?
  • All unit tests/integration tests are included in this pull request? If you think adding test cases is unnecessary, please describe why and cross out this line.
  • The documentation for this pull request is enough? If you think adding documents for this pull request is unnecessary, please describe why and cross out this line.

@hakuturu583 hakuturu583 added wait for regression test bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 and removed wait for regression test labels Sep 17, 2024
@hakuturu583 hakuturu583 changed the title Fix/fix eigen variable definition (DO NOT MERGE!!!) Fix/fix eigen variable definition Sep 18, 2024
@hakuturu583 hakuturu583 marked this pull request as ready for review September 18, 2024 03:10
@hakuturu583
Copy link
Collaborator Author

Fixing release action is working on #1248

@hakuturu583 hakuturu583 merged commit b189919 into master Sep 19, 2024
10 checks passed
@github-actions github-actions bot deleted the fix/fix-eigen-variable-definition branch September 19, 2024 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 wait for regression test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The position of kinematic_state topic is not updated
3 participants