Re: arend_1_34.1: Re: [gnugo-devel] 3.1.33 VC inconsistencies

[Trevor Morris]

This patch does make aa_attackpats identical.
It also makes consistent owl_rot:66 and 13x13:63.
Great work.

I'm kicking off a full regression locally with this patch now.

I'm in favor of adding this to CVS, as it would be very nice to have the 3.2
engine behave identically cross-platform - remember, we'll continue to get
bug reports on it for a good 6-9 months probably.

Dan: Please drop me a line if you add this to CVS, and I'll run a full 
regression
on the latest CVS for a VC build.

Other comments sprinkled below...

-Trevor

At 07:02 PM 4/20/2002 +0200, Arend Bayer wrote:
>On Sat, 20 Apr 2002, Trevor Morris wrote:
>
>> I've noticed one thing so far that's odd.  Namely, aa_attackpats.c is 
>> different
>> btw. VC & cygwin builds.  You can see the VC version at:
>>   
>> http://www.public32.com/pcolon/builds/gnugo/2002-4-19/gnugo/patterns/aa_attackpat.c
>> 
>> The only other files that differ are eyes.c and the joseki files, but I 
>> believe
>> this is CR/LF differences only.  The differences in aa_attackpat.c look
>> substantive.
>
>I've done a quick web search, and it seems that for "release builds" (which
>Trevor is using IIRC) the VC compiler will inline functions if they seem
>suitable. (See 
>http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore/html/_core_.2f.Ob.asp
> )
I see the same behavior in debug builds, so this is probably not the cause.

>Then gg_normalize_float may well get inlined. As Gunnar pointed out
>correcting my floating point remarks, this function may no longer be safe
>in this case.
>
>This could explain the differences in aa_attack_pats.c, and although I
>couldn't quite parse the file, the diff seems to come from choosing
>different anchors. It should be corrected by the patch below.
>
>Are you sure that there were no differences in owl_*pat.c? I get a different
>sorting in the list of patterns if I break at the same point. 
Yes, I'm quite sure.  They were identical.

>This can have 2 reasons:
> * DFA matching found the patterns in a different order, most likely by
>   a different database.
> * There might be some problem in the sorting in get_next_move_from_list;
>   this does also use comparisons between identical floating point numbers.
>   However, there is absolutely no computations done with these -- apart
>   from a nonsense (int) conversion which is also eliminated in the
>   patch below.
>
>
>Btw, as I see it there are 2 reasons to worry about platform dependencies:
> 1. They might be caused by bugs.
> 2. They are annoying for us when tuning against regressions.
>
>However, if we have eliminated 1., I don't think they are a problem for
>the stable release. In other words, in case this patch resolves the
>VC inconsistencies, I don't actually see the need to apply it before 3.3
>(and I would feel safer not to be responsible for such a very-last-minute
>change...).

As I said above, I think it's worth having the engines perform identically,
especially when we're really so close.