[Freeciv-Dev] Re: directional system: more magic code cleanups

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Sep 25, 2001 at 03:11:26AM -0400, Ross W. Wetmore wrote:
> At 09:12 AM 01/09/24 +0200, Raimar Falke wrote:
> >On Mon, Sep 24, 2001 at 01:19:14AM -0400, Ross W. Wetmore wrote:
> >> I notice you left the two normalize_map_pos() operations in for the
> >> Ross code, but neglected to add them for the others. This will change
> >> the profiling results somewhat, as function calls will probably
> >> dominate the time, and even the extra double compare in a macro
> >> is probably significant here.
> >> 
> >> Since these are there really to handle the truly oddball cases where
> >> the src and destination are separated by more than a full map in the
> >> wrapped case, and none of the others are checking for this it is
> >> redundant.
> 
> >I haven't made any change to normalize_map_pos calls.
> 
> Then your profiling numbers are wrong as pointed out to you above. You
> are comparing apples and oranges if you don't remove them in one or add 
> them to the other so that both are either handling the delta_x > map.xsize
> cases or they both aren't. 

I haven't used speed as an argument. Or have I? If the algorithms are
correct we can take a look at speed.

> Your code is buggy under these conditions, but they are presumably not
> normal behaviour and there is a judgment call as to whether the check is
> warranted or not.
> 
> >> The code appears to have been reformatted as well since I generally
> >> lineup the things like if clauses in ways that enhance similarities
> >> rather than obscure them. I don't believe the multi-line if's are 
> >> broken in quite the way that the sample code was in when sent to you.
> >> So I take some offense at your "readability" comments.
> >
> >Yes I reformatted the code. Yes I changed two things: MAP_WRAP_[XY] in
> >your code and one error in my code. 
> 
> If you change someone's code by reformatting it, then you should not
> make comments about how unreadable you have made it vs how readable
> your unformatted code looks. 
> 
> That is only fair.

Ok. However every piece of code which will get into cvs will be
reformatted.

> >> I suspect, that your "bugs" are really just the fact that the scalar
> >> product cases don't prefer the diagonal move when diagonal or cartesian
> >> are equivalent.
> >
> >Ross it doesn't work this way. I tested the code and showed that the
> >result of your method is different than 3 others. I expected from you
> >that you take a look and either prove that your method is right and
> >the other three are buggy and tell why this is so OR you post a fixed
> >version of your code OR you show that there are really different
> >results possible. BUT NOT a "I suspect".
> 

> I think I have proved how my method works and how it balances the game
> play in many more emails than I want to remember, so you will have to 
> come up with a better monkey argument than this. It has also been running 
> and tested for far longer.

Your testing was wrong. See below.

> Since you haven't posted this data to show what the differences are, 
> (and from your code it looks like you have arranged to make a single
> difference fill up screeds of screen or file real estate without giving
> any really useful information)

Yes this was bad. I first compared them, found that your method differ
from the others and then I added some loops to get some profiling
data.

> , and you haven't done any analysis except say that the two methods
> are different, it is a little difficult to refute such vaporware
> claims which don't even have credibility going for them.

The code speaks for itself. There are two methods used (scalar product
and my tan method) and both differ from your method. This should be
hint enough for you to take a look at it.

> Moreover since it is just "different" from your routines, you really 
> should have looked more closely at the characteristics of your algorithm
> as I have done with mine and shared with the mailing list. Your claims 
> of "buggy" seem to be more wishful thinking than fact. 
> 
> If you are just out to prove something, then make sure your proof is 
> solid :-).

Ok. From a test game:

2: src=(43,4) dest=50,7)
2:   orig=SE, jason=SE, ross=E, raimar=SE, raimar2=SE

So we see that diff_x=7 diff_y=3. So we have a gradient of
m=diff_y/diff_x=3/7.

Now comes the tricky part where your error is: which gradient is the
border between the SE part and the E part? I think your answer would
be 0.5. Probably because m=1 divides the quadrant. And 1/2 would
quarter the quadrant. THIS IS WRONG. The correct gradient is
tan(pi/8)=0.4142... It is now no surprise that the gradient from above
m=3/7=0.42857... is between 0.4142 and 0.5.

Has this showed up during your testing? No. This is not
surprising. The example of the useless code in goto_zoc_ok should us
teach that you have to "really" test a method which can not easily be
tested by the gui.

Did I expect that you find this error by yourself after pointing out
that there are differences (I didn't say error here)? Yes.

So if you come up with a new version of your method I will be happy to
test and profile it.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 1 + 1 = 3, for large values of 1