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(simple_sensor_simulator): fix eigen variable definition #1313

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

MertClk
Copy link
Contributor

@MertClk MertClk commented Jul 5, 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.

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/319

@MertClk MertClk force-pushed the fix/fix-eigen-variable-definition branch from c0b59b9 to 0894570 Compare July 9, 2024 08:35
@hakuturu583 hakuturu583 added bump minor If this pull request merged, bump minor version of the scenario_simulator_v2 wait for regression test labels Jul 19, 2024
@hakuturu583
Copy link
Collaborator

I am now posting regression test job.

@hakuturu583
Copy link
Collaborator

I found that release action was failed because of my fault.
So, I ignore failure of release action.

@hakuturu583 hakuturu583 self-requested a review July 19, 2024 09:22
@hakuturu583 hakuturu583 added bump patch If this pull request merged, bump patch version of the scenario_simulator_v2 and removed bump minor If this pull request merged, bump minor version of the scenario_simulator_v2 labels Jul 19, 2024
@hakuturu583
Copy link
Collaborator

Now start running regression test. (Sorry, this is internal link.)

@hakuturu583
Copy link
Collaborator

Sorry, we are running regression test but it was failed because of something wrong happened in our regression test system.
Now we are try to recovering...
Please wait.

@hakuturu583
Copy link
Collaborator

hakuturu583 commented Sep 18, 2024

I feel very sorry to you.

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 (#1324) and carry out verification and merging.

@hakuturu583
Copy link
Collaborator

Fixing release action is working on #1248

@hakuturu583 hakuturu583 merged commit 4a6adbe into tier4:master Sep 19, 2024
45 of 53 checks passed
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
2 participants