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 iterator bugs in path optimizers #77

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

Kazfyx
Copy link
Contributor

@Kazfyx Kazfyx commented Dec 13, 2023

In the simple optimizer and advanced optimizer, some iterators are increased in the loop body. But at the end of each loop, these iterators will be increased again by the for-loop structure. When the program is running in debug mode, an assertion often occurs that an iterator is behind the end iterator of a VectorList. I think this is not the real intention of the developers, and it would not get a correct optimization result.

@rickertm
Copy link
Member

Thank you for reporting this issue and the suggested fixes. This was introduced during the refactoring in 5064ded and is indeed not the intended behavior.

The refactoring to for loops and the updated init statements do not have to be reverted, just the iteration expression, otherwise this fix looks good to me. Would you mind updating this accordingly?

@Kazfyx
Copy link
Contributor Author

Kazfyx commented Feb 18, 2024

Thank you for reporting this issue and the suggested fixes. This was introduced during the refactoring in 5064ded and is indeed not the intended behavior.

The refactoring to for loops and the updated init statements do not have to be reverted, just the iteration expression, otherwise this fix looks good to me. Would you mind updating this accordingly?

As iterator increasing is fully controlled in the loop body, the iteration expression must be empty. I may write something like this:

for (VectorList::iterator i = path.begin(), j = ::std::next(i), k = ::std::next(j); i != path.end() && j != path.end() && k != path.end(); )
{
    ...
}

This is not a typical usage of for-loop, and I think it has no advantage compared whith a while-loop. Of course, if you insist on for-loop, I can update it.

@rickertm
Copy link
Member

The iteration expression should be empty in this case, as in your example. The behavior of the two variants is identical, but the scope of the loop variables is limited in the case of the for loop.

@rickertm rickertm merged commit 0b37972 into roboticslibrary:master Feb 20, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants