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

[RST-2437] Ensure that all variables are updated #103

Merged
merged 3 commits into from
Sep 3, 2019

Conversation

svwilliams
Copy link
Contributor

Ensure that all variables are updated when the update_variables flag is set. Previously, only variables involved in motion model segments that needed to be generated were being updated. Variables involved in previously generated segments were left untouched. This could result in incorrect initial state guesses being used for those variables.

As an example, imagine a sensor transaction that connects two adjacent states at T1 and T2, and a second transaction that connects T1 to T3. If both of these transactions are added to the graph at the same time:

  • Transaction1 is processed. A motion model segment is generated from T1 to T2. Since states T1 and T2 are involved in the newly generated motion model segment, their states are updated. No other states are involved in this transaction, so all states have been updated.
  • Transaction2 is processed. A motion model is generated from T2 to T3. Since a motion model was previously generated from T1 to T2, the motion model does not need to regenerate that segment. Since states T2 and T3 are involved in the newly generated motion model segment, their states are updated. However, the transaction also involves the state at T1. It was not involved in the generated motion model segment, so its state has not been updated. The original state from Transaction2 overwrites the updated state from Transaction1. This can lead to a poor initial guess being sent to the optimizer.

With this PR, if the update_variables flag is set, any variable from a previously generated motion model segment that is also involved in the current transaction is updated from the value stored in the motion model history.

std::set<ros::Time> augmented_stamps(stamps.begin(), stamps.end());
auto first_stamp = *augmented_stamps.begin();
ros::Time last_stamp = *augmented_stamps.rbegin();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe do auto last_stamp as you do with first_stamp

if (std::any_of(
transaction_variables.begin(),
transaction_variables.end(),
[&variable](const fuse_core::Variable& input_variable)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In C++14 you can do:

[variable_uuid = variable->uuid()](const auto& input_variable)
{
  return input_variable.uuid() == variable_uuid;
}

@svwilliams svwilliams merged commit 4278a83 into devel Sep 3, 2019
@svwilliams svwilliams deleted the RST-2437-missing-initial-guesses branch September 3, 2019 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants