Skip to content

Commit

Permalink
Bug fix in task start/finish lines
Browse files Browse the repository at this point in the history
A pilot using xcsoar software a few days ago reported a problem with the task startline not being accurate at the borders.
It seems that the error depends on the fact that, as the pilot explains, the start line is not 90 degrees exactly to the target point.
We assume the same can happen with the finish line.

This patch is fixing the calculation of the angle, by applying the Great Circle convergency which was not taken into account.
  • Loading branch information
pventafridda committed Jul 25, 2012
1 parent 314916d commit f3fe24f
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions Common/Source/Calc/Task/TaskUtils.cpp
Expand Up @@ -46,6 +46,13 @@ void RefreshTaskWaypoint(int i) {
&Task[i].Leg,
&Task[i].InBound);

// Apply Great Circle convergency
double chlon = (WayPointList[Task[i-1].Index].Longitude - WayPointList[Task[i].Index].Longitude) * DEG_TO_RAD;
double medlat= (WayPointList[Task[i-1].Index].Latitude + WayPointList[Task[i].Index].Latitude) / 2;
double conv= (chlon * sin(medlat*DEG_TO_RAD)) * RAD_TO_DEG;
Task[i].InBound -= conv;


Task[i-1].OutBound = Task[i].InBound;
Task[i-1].Bisector = BiSector(Task[i-1].InBound,Task[i-1].OutBound);
if (i==1) {
Expand Down

8 comments on commit f3fe24f

@pventafridda
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no overhead to do this spare calculation, because this function is called only when creating or changing the task.
Distance from A to B or from B to A does not change, but the bearing is not the same, it is changing depending on where the points are, and how far they are from each other. This was the problem, known in navigation as "great circle convergence", and the solution is to compensate it. This is in all navigation exams for pilots, in the US. We apply exactly the same calculus, with the exception that instead of using a simple "standard mean latitude" we calculate full mean latitude, for a 1/100000 degree precision.

@MaxKellermann
Copy link
Contributor

Choose a reason for hiding this comment

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

[NOTICE: ALL OF THESE CALCULATIONS ARE WRONG BECAUSE HE IS FORGETTING TO CONVERT TO RADIANS AND BACKWARDS]

Nice try, but this patch doesn't work either. You're trying hard to avoid merging my patch, and you're making one mistake after another in your failed attempts to replicate my bug fix while avoiding my code. Anyway, good you followed my suggestion to revert your horrible first approach.

Let's say you fly from (70,0) to (70,180) [lat,lon], i.e. beyond the north pole. DistanceBearing() will set InBound to 0 deg, because you're flying north, but it really should have been 180 deg.

chlon = 0 - 180 = 180 deg
medlat = (70 + 70) / 2 = 70 deg
conv = chlon * sin(medlat) = 180 * sin(70) = 169.1
InBound -= conv => InBound = 0 - 169.1 = 190.9

This is wrong by more than 10 degrees, which is more than the error that was reported by Peter Scholz.

Let's take the example from Peter's bug report (N50°22'58" E008°27'28" to N50°13'17" E007°38'36"):

InBound = 253.086838
correct InBound according to my patch = 252.460184
chlon = (8.45777-7.64333) = 0.81444
medlat = (50.3827+50.2214)/2 = 50.30205
conv = chlon * sin(medlat) = 0.81444 * sin(50.30205) = 0.81444*0.03655939351223691 = 0.02977543245210624
InBound -= conv => InBound = 252.4304085675479

This is an improvement, but it's not the correct result. It is safe to say LK8000 still miscalculates the observation zone angle.

So this commit boils down to:

  • it's still wrong (but less so)
  • it's obscure code, hard to understand
  • code is bigger
  • wastes more CPU cycles

I suggest you revert this one, and merge my solution. My code is a simple and clean solution, everybody understands it, and it's correct.

@pventafridda
Copy link
Member Author

Choose a reason for hiding this comment

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

Your numbers are wrong.
Using the coords correctly inserted by hand, just to be sure.
MY A:lat=50.382783 lon=8.457783 B:lat=50.221383 lon=7.643333 Leg=60565.711941, InBound=253.076982
YOURS A:lat=50.221383 lon=7.643333 B:lat=50.382783 lon=8.457783 Leg=60565.711941, InBound=252.45032

Now applying the convergency formula:
change in longitude is 0.814437 radiant equiv is 0.01421460 etc
mean latitude 50.30205
sinmedlat 0.769422409188

Convergency in radiants:
sinmedlat * chlon
0.769422409188 * 0.0142146072001 = 0.0109370373176
convert it to degrees, conv=0.6266460

Summary:
YOUR Angle: 252.45032
MY angle - convergence = 253.076982 - 0.626646 = 252.45036

the difference is 0.00004 degrees

thanks for playing, chess mate

I DID NOT follow your suggestions, au contrraire: don't take credits when you have none. You dont have any merit in this patch.
I just used peter sholz bug advise and applied the correct fix to fix the angle. You did not even know what this patch is doing.
You had bet we had no chance other than merging your code or copying the solution of inverting A with B point.
And you were damn wrong, Batman.

@pventafridda
Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, taking the exception of north and south poles for the convercency formula is something only you and Santa Claus could have thought about. We could deal with that, if needed, just handling the exception. Intentionally we dont, because nobody can fly with a glider over the north pole. Nice try .

@MaxKellermann
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not about winning or losing a game. You're being childish, you seem unable to do a technical discussion, your mind seems to be blurred by your hatred against me.

So my example about the north pole is about demonstrating that your formula is only an approximation that works within certain limits, but is not 100% correct, while my approach indeed is 100% correct. Taking extreme examples is a common and simple way of demonstrating the (in)correctness of a solution. Whether you're really going to fly with your glider around the north pole is not relevant. Thinking it is only suggests that you're unable to do an abstraction of that example.

Your patch still has an error, bloats the code, adds CPU usage, and obfuscates the code. You chose an inferior solution just because the perfect solution was submitted by me. You will always have to carry this hideous code until you give in and merge the "perfect" solution from me.

By submitting my patch, in an attempt to improve LK8000, I actually detoriated LK8000, because my submission had the power to make you choose the inferior solution. That was not my intention. I have the power to detract LK8000 from being "perfect" by submitting a perfect solution. See, that's what I can do if I really want to: if I really want to join your game of perfidy, I have great powers, I win because I make you do things, but you cannot make me do things. But I don't want to.

@pventafridda
Copy link
Member Author

@pventafridda pventafridda commented on f3fe24f Jul 26, 2012 via email

Choose a reason for hiding this comment

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

@MaxKellermann
Copy link
Contributor

Choose a reason for hiding this comment

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

An excellent demonstration of different developer philosophy.

  • If XCSoar uses a wrong reference in a formula, we change the code to use the correct reference.
  • If LK8000 uses a wrong reference in a formula, you continue using the wrong reference, but add an approximation formula to reduce the error.

So long.

@pventafridda
Copy link
Member Author

Choose a reason for hiding this comment

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

Kellermann, do you want to debate assertions or are you only listening to
yourself, as usual?
The difference in your example, where you miserably miscalculated
convergence value, was 0.00004 degrees
That is 4E-5 . That is It is 10.000 (ten thousands) more accurate than what
xcsoar and lk8000 has been using for 8 years, up to a week ago.

The 0.00004 degrees error, which is a rounding error, is a very small price,
like a penny in the EU balance, that anyone would gladly pay to keep you
off.
So long, to you and your can of worms.

Please sign in to comment.