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

in the function "calculateHSignature" skip really close obstacles #90

Closed
chenxianbo opened this issue Aug 14, 2018 · 1 comment
Closed

Comments

@chenxianbo
Copy link

void calculateHSignature(BidirIter path_start, BidirIter path_end, Fun fun_cplx_point, const ObstContainer* obstacles)
{
.....
//if (diff.real()!=0 || diff.imag()!=0)
if (std::abs(diff)<0.05) // skip really close obstacles
Al /= diff;
else
continue;
}
.....
}

when abs(diff)<0.05, we get it calculate Al, but the note say " skip really close obstacles", it is opposite

@croesmann
Copy link
Member

thanks a lot, you're right. fixed in kinetic-devel. I will port this to other distributions as well.

croesmann added a commit that referenced this issue Aug 14, 2018
(cherry picked from commit 682ad24)
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 21, 2019
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 21, 2019
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 21, 2019
Needed to pair with the fix for rst-tu-dortmund#90
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 21, 2019
Needed to pair with the fix for rst-tu-dortmund#90
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 21, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 22, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Nov 26, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 2, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 2, 2019
First, this upstream change corrected a bug in the denominator
calculation of Al, but broke the H-signature by making them degenerate
to near-zero values for non-trivial obstacle count:

682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")

Then, my commit fixes the numerator to be a power function (as it was in
an original commented out line of code), making the signatures
functional again:

9bb9cf5 ("h_signature: Revert to power function for f0 (numerator)")

The result of both of those commits is functional but the code executes
about 5 to 10 times slower than before either of those changes!
Although the math seems wrong, in practice, the code worked sufficiently
correctly before those fixes.

This commit effectively reverts both of them for performance reasons.

TODO:
* Research why this seems to work even though it differs from the
  research paper substantially.
* Research alternative ways to improve performance, such as is
  pre-calculating the denominator of Al and re-using for all obstacles.
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 3, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 3, 2019
First, this upstream change corrected a bug in the denominator
calculation of Al, but broke the H-signature by making them degenerate
to near-zero values for non-trivial obstacle count:

682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")

Then, my commit fixes the numerator to be a power function (as it was in
an original commented out line of code), making the signatures
functional again:

9bb9cf5 ("h_signature: Revert to power function for f0 (numerator)")

The result of both of those commits is functional but the code executes
about 5 to 10 times slower than before either of those changes!
Although the math seems wrong, in practice, the code worked sufficiently
correctly before those fixes.

This commit effectively reverts both of them for performance reasons.

TODO:
* Research why this seems to work even though it differs from the
  research paper substantially.
* Research alternative ways to improve performance, such as is
  pre-calculating the denominator of Al and re-using for all obstacles.
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 3, 2019
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
howardcochran pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Dec 3, 2019
First, this upstream change corrected a bug in the denominator
calculation of Al, but broke the H-signature by making them degenerate
to near-zero values for non-trivial obstacle count:

682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")

Then, my commit fixes the numerator to be a power function (as it was in
an original commented out line of code), making the signatures
functional again:

9bb9cf5 ("h_signature: Revert to power function for f0 (numerator)")

The result of both of those commits is functional but the code executes
about 5 to 10 times slower than before either of those changes!
Although the math seems wrong, in practice, the code worked sufficiently
correctly before those fixes.

This commit effectively reverts both of them for performance reasons.

TODO:
* Research why this seems to work even though it differs from the
  research paper substantially.
* Research alternative ways to improve performance, such as is
  pre-calculating the denominator of Al and re-using for all obstacles.
12tbuchman pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Aug 6, 2021
A previous bugfix for Issue rst-tu-dortmund#90 corrected the denominator of Al which
was generally being left equal to (1, 0) before that fix. After that
fix, the denominator generally becomes very large if there are more than
a few obstacles in the scene (e.g. With ~30 obstacles, the denominator
was around 10^17)

In what was likely an earlier attempt to fix that caused by the
incorrect denominator calculation, the original power function used in
the numerator (f0) had been replaced by a simple multiplication.
However, now that we are calculating the denominator correctly while
still using only a small numerator, all the H-signatures have become
extremely small (on the order of 10^-16 with around 30 obstacles in my
tests).  Thus, ::isEqual() method always returns true so we never
explore any alternate plans, rendering the HCP ineffective.

This change restores the original calculation of f0 thus making it large
enough that the huge denominator will not dengenerate the H-signature to
near zero. In my tests, this makes the HCP function correctly again.

Fixes: 682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")
12tbuchman pushed a commit to BadgerTechnologies/teb_local_planner that referenced this issue Aug 6, 2021
First, this upstream change corrected a bug in the denominator
calculation of Al, but broke the H-signature by making them degenerate
to near-zero values for non-trivial obstacle count:

682ad24 ("bugfix in calculateHSignature. Fixes rst-tu-dortmund#90")

Then, my commit fixes the numerator to be a power function (as it was in
an original commented out line of code), making the signatures
functional again:

9bb9cf5 ("h_signature: Revert to power function for f0 (numerator)")

The result of both of those commits is functional but the code executes
about 5 to 10 times slower than before either of those changes!
Although the math seems wrong, in practice, the code worked sufficiently
correctly before those fixes.

This commit effectively reverts both of them for performance reasons.

TODO:
* Research why this seems to work even though it differs from the
  research paper substantially.
* Research alternative ways to improve performance, such as is
  pre-calculating the denominator of Al and re-using for all obstacles.
This issue was closed.
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

No branches or pull requests

2 participants