search

locusrobotics / robot_navigation

Spiritual successor to ros-planning/navigation.
450 stars 148 forks source link

Fix math in README #37

Closed EwingKang closed 5 years ago

EwingKang commented 5 years ago

According to the explanation of the rest of the steps, the quadratic equation to the euclidean potential distance should be h^2. Add some annotation about the Taylor series expansion. It is expanded at P(C)=P(A)+h according to the plot.

DLu commented 5 years ago

Thank you for the update. It is nice to know that someone is examining the calculations, much less with any mathematical rigor.

I'm relatively certain you first change is correct, though I don't have the notes in front of me. However, I did update the url syntax a little.

For the second update, I think the value is slightly different. The inequalities in the plot show our assumptions, i.e. that P(A) <= P(C) and P(C) - P(A) < h. If I recall correctly, the taylor series was calculated for the center of that window, i.e. P(C) + h/2. Does that mesh with your thinking, or am I remembering it wrong.

EwingKang commented 5 years ago

@DLu I think you are correct. In fact, after I submit a PR, I tried to derive the equation my self again and couldn't get the same result (took some time for me to get my Taylor series right :P ). I'll try again at P(C)+h/2. Sorry for the non-accurate PR.

DLu commented 5 years ago

No apologies necessary. It's the great thing about open source that we have the chance to discuss.

Let me know how your additional attempt goes. We can add more rigor to the explanation so the next person doesn't hit the same bumps.

DLu commented 5 years ago

Did you make any progress on re-deriving the equation @EwingKang ?

EwingKang commented 5 years ago

@DLu this is embarrassing, I'm having some crazy work schedule for the past two weeks. I will dedicate some time this weekend, hopefully to get some definate result.(or unable to do the difficult math😛)

EwingKang commented 5 years ago

Appearantly, there are indeed errors within the readme. Delta should be defined as: delta = (P(C) - P(A)) / h Instead of delta = beta / h, beta = -(P(A) + P(C)). This caught me off guard and get me confused for a long time until I checked the plot and see that D has a different definition in the equation. My derivation also produced the same mathematical form.

Unfortunately, My derivation of a 2nd degree Taylor expansion will not yield the result coefficient as stated. In fact, it is quite off according to the plot. I've tried to get my math as accurate as possible, but there might still be some error. I do believe the original coefficient is correct since the graphics shows the good approximation within the region.
IMAG0558

Additionally, I've derived the 3rd degree Taylor and find that the coefficient gets closer to the ones in the readme. I suspect that either my math is wrong, or the coefficient is found by a taylor series of much higher degree, even infinite degree since the coefficient seems to be a formulatable-converging series.

Apart from my inability to find the correct coefficient, I do believe the current is commit is corrct after the delta fix.

DLu commented 5 years ago

Apologies for the delays @EwingKang...it took me quite awhile to go through all the math.

You are definitely correct that I was defining delta incorrectly, and I sincerely apologize for the dead ends it may have taken you down. I'm going to merge this and then open a new PR with the full derivation written out.

EwingKang commented 5 years ago

@DLu No need for apologies, its quite interesting for me to stretch my math nerves. I still haven't figure out where I got the expansion wrong. Definitely will look into your equations for clues. Thanks for keeping the communities alive!

Edit I checked your math, looks quite nice. Seems like my derivation didn't help, sorry for the confusing math :stuck_out_tongue: . Couldn't wrap my head around the expansion with change of variable part haha.