.com.unity Forums

.com.unity Forums (http://forum.shrapnelgames.com/index.php)
-   Dominions 2: The Ascension Wars (http://forum.shrapnelgames.com/forumdisplay.php?f=55)
-   -   2.08 and Incompatible Battle Reports (http://forum.shrapnelgames.com/showthread.php?t=17878)

AStott February 26th, 2004 12:34 AM

Re: 2.08 and Incompatible Battle Reports
 
Eh... why bother with all the complicated methods of ensuring the calls happen in the right order. Instead, change:
if penetration+2d6 < MR+2d6

To:
if penetration < MR+2d6-2d6

Since both of the random calls are on the same side of the equation, the defined precendence order will ensure they are called in a consistent manner.

Arryn February 26th, 2004 12:47 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by AStott:
Eh... why bother with all the complicated methods of ensuring the calls happen in the right order. Instead, change:
if penetration+2d6 < MR+2d6

To:
if penetration < MR+2d6-2d6

Since both of the random calls are on the same side of the equation, the defined precendence order will ensure they are called in a consistent manner.

<font size="2" face="sans-serif, arial, verdana">The problem comes in that the compiler's optimization will substitute the same call to the random number function for both die rolls. It won't make the two rand() calls the coders intend. What it does is make one call and plug the same value into both places. The optimizer does not know that in this circumstance, two calls to the same function do not return the same value.

Peter Ebbesen February 26th, 2004 01:17 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by Arryn:
The problem comes in that the compiler's optimization will substitute the same call to the random number function for both die rolls. It won't make the two rand() calls the coders intend. What it does is make one call and plug the same value into both places. The optimizer does not know that in this circumstance, two calls to the same function do not return the same value.
<font size="2" face="sans-serif, arial, verdana">That would be an exceedingly poorly written optimizer. I certainly did not understand that as the original problem. I am certain that any decent compiler would make the two rand calls.

As I understood it, the real problem was that in the expression "a < b" the order of evaluation of "a" and "b" is undefined [Ansi-C, and its derivatives, with a few exceptions only specifies the order of evaluation for operators, not for sub-expressions]. Either side "a" or "b" can be evaluated first.

Given that the seed is the same, assume the RNG will return R1 and R2 over the next two calls in that order. Then the inequality:

(penetration+2d6 < MR+2d6)

can legally be compiled such that the evaluation is either of the following:
penetration+R1 < MR+R2 (left hand side evaluated first)
penetration+R2 < MR+R1 (right hand side evaluated first)

- Which may or may not give different return values.

Changing the inequality as AStott suggested, to
penetration < MR+2d6-2d6
would not necessarily solve the problem either, as the order of evaluation of the two 2d6 function calls is not defined either. (The order of the addition and subtraction is, but not the order of evaluation of the sub-expressions)

If you are in doubt, split such expressions over multiple commands independently evaluated. A bad optimizer may still hurt you, but at least you won't be bitten by the "undefined evaluation order", which is nobody's fault but your own. Or, to quote Kernigham & Ritchie:

Quote:

The moral is that writing code that depends on order of evaluation is a bad programming practice in any language. Naturally, it is necessary to know what things to avoid, but if you don't know how they are done on various machines, you won't be tempted to take advantage of a particular implementation.
<font size="2" face="sans-serif, arial, verdana">

[ February 25, 2004, 23:23: Message edited by: Peter Ebbesen ]

Arryn February 26th, 2004 01:36 AM

Re: 2.08 and Incompatible Battle Reports
 
A cogent, well-stated reply, Peter. The quote by K&R was what I tried to allude to in an earlier posting.

BTW, it's not so much that the opitimizer may be "bad" than that it may be using "overly aggressive" choices. I remember the Borland compiler dev team in the early 90s having many headaches over just how far they should go. In those days, Borland and MS kept trying to out-do each other via how powerful their optimizations were. After a few cycles of this we started seeing cases of too much optimization.

A careful review of what all the default optimization switches do should be undertaken by anyone that's serious about code-writing, especially cross-platform code, to avoid potential pitfalls. Hell, one should also look at the linker's switches too.

alexti February 26th, 2004 01:43 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by Arryn:
The problem comes in that the compiler's optimization will substitute the same call to the random number function for both die rolls. It won't make the two rand() calls the coders intend. What it does is make one call and plug the same value into both places. The optimizer does not know that in this circumstance, two calls to the same function do not return the same value.
<font size="2" face="sans-serif, arial, verdana">Consider the following:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> void print_blank_line { printf(&quot;\n&quot;); }

void print_2blank_lines()
{
print_blank_line();
print_blank_line();
}</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">Do you think compiler/optimizer will call print_blank_line just once? The similar situation will happen in the following:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> std::vector&lt;int&gt; x;

void foo()
{
x.push_back(1);
x.push_back(1);
}</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">Will push_back be called only once (parameters are the same)?

If any compiler does make one call instead of 2 in these cases, you're not likely to build anything usable with it. So it's safe to assume that any common compiler doesn't have this problem.

alexti February 26th, 2004 01:47 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by Taqwus:
*scratches head*
'volatile' may also help. The MSDN C++ Language Reference states, "Objects declared as volatile are not used in optimizations because their value can change at any time"

<font size="2" face="sans-serif, arial, verdana">Volatile won't help here, the idea behind volatile is that if you need to read/write into the some variables of one process from another process (after passing address by some IPC means), compiler/optimizer won't know that you plan to use the variable in such a way and may put it into the register. So volatile is just used to prevent it from happening.

Arryn February 26th, 2004 02:01 AM

Re: 2.08 and Incompatible Battle Reports
 
Alexti, your examples are not the same. In the first example: 1) they have void returns and invoke I/O, which the optimizer treats differently. 2) the statements are on separate lines, and IIRC there are various scoping rules to what the compiler will attempt to "consolidate" when it goes to generate machine code.

For the second example: it depends on the switches used. I have seen (though not recently) compilers that would, indeed, consolidate those two statements when you viewed generated assembler code.

However, the most obvious thing you said that bears careful review is the statement "... it's safe to assume ...". It's never safe to assume anything. That's the first step towards making mistakes ...

We've flown the Space Shuttle in cold weather before, and it's only a few degrees colder now, the Challenger should be okay.

It's only a 2-pound chunk of foam. How can that damage reinforced carbon fiber? It probably shattered into dust on impact with the Columbia's wing.

The list of assumptions people make that lead to bad results has a long and inglorious history.

alexti February 26th, 2004 02:02 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by Johan K:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">quote:</font><hr /><font size="2" face="sans-serif, arial, verdana">Originally posted by Pocus:
I have often battle inconstancies message now, with solo play on windows platform.

<font size="2" face="sans-serif, arial, verdana">Bah, can't you let me be happy for more than 1 hour and 13 minutes. http://forum.shrapnelgames.com/images/icons/tongue.gif </font><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">To Johan K:

I guess you have few more places when the comparison looks like
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> if (x + 2d6 &lt; y + 2d6)</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">To eliminate this problem you could make the function:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> int random_compare(int x, int y, int nx, int ny)
{
int x1 = x + nx*illwinter_dice();
int y1 = y + ny*illwinter_dice();
return (x1 - y1);
}</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">and use comparison
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> if (0 &gt; random_compare(x,y,2,2))</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">Finding all the places where the dice function is used and replacing it with the new comparison is not difficult, but the problem is that if you make a small cut-and-paste mistake somewhere, how do you find it?

I can think only about one method to ensure that everything is changed correctly. If you can save battle progress status on disk after every round, you can make "etalon" saves on the current Version and rerun the test on a modified Version, comparing battle status after every turn. If there's a difference you'd be able to go through debugger to find out where it comes from. (In the example I've given you'd have to run on Linux, so that the original evaluation order matches the one implemented in a new function). The drawback is that some obscure conditions may not get tested at all.

Alternative method is to write a program which will automatically do conVersion http://forum.shrapnelgames.com/images/icons/icon7.gif

Either way it doesn't look like an easy fix :-(
But I still hope that you don't have that many places where dice function is called more than once in the expression, so that those can be examined manually http://forum.shrapnelgames.com/images/icons/icon7.gif

alexti February 26th, 2004 02:14 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by Arryn:
Alexti, your examples are not the same. In the first example: 1) they have void returns and invoke I/O, which the optimizer treats differently.

<font size="2" face="sans-serif, arial, verdana">How the optimizer would know about IO? printf is just some function residing in some library to which optimizer had no access. It only can see the header, where there's nothing that says that the function contains IO.

Quote:


2) the statements are on separate lines, and IIRC there are various scoping rules to what the compiler will attempt to "consolidate" when it goes to generate machine code.

<font size="2" face="sans-serif, arial, verdana">Some Languages consider line feeds as a language element, but not C/C++.
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> f(); f();</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">and
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> f();
f();</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">are the same thing.


Quote:

However, the most obvious thing you said that bears careful review is the statement "... it's safe to assume ...". It's never safe to assume anything. That's the first step towards making mistakes ...

<font size="2" face="sans-serif, arial, verdana">You have to assume that the compiler works correctly (according to the standard) until proven otherwise, how are going to write any code otherwise?

Consider the following:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> int a = 1;
foo(a);</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">What if the compiler generate wrong code for assignement? Ok, here is an improvement:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> int a = 1;
if (a != 1)
ERROR(&quot;!!!&quot;);
foo(a);</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">But what if the code for comparison is wrong too? No problems:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> int a = 1;
if (a != 1 || (a-1))
ERROR(&quot;!!!&quot;);
foo(a);</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">But what if operator or (||) produces wrong code too?

Well, it's clear where it's going...

Peter Ebbesen February 26th, 2004 09:16 AM

Re: 2.08 and Incompatible Battle Reports
 
Quote:

Originally posted by alexti:

</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> if (x + 2d6 &lt; y + 2d6)</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">To eliminate this problem you could make the function:
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> int random_compare(int x, int y, int nx, int ny)
{
int x1 = x + nx*illwinter_dice();
int y1 = y + ny*illwinter_dice();
return (x1 - y1);
}</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">and use comparison
</font><blockquote><font size="1" face="sans-serif, arial, verdana">code:</font><hr /><pre style="font-size:x-small; font-family: monospace;"> if (0 &gt; random_compare(x,y,2,2))</pre><hr /></blockquote><font size="2" face="sans-serif, arial, verdana">

<font size="2" face="sans-serif, arial, verdana">A nice idea, though it fails in the execution. 2d6 is not shorthand for two times the result of a d6, but for the openended result of rolling 2 d6'es (openended: sixes are rerolled and added potentially ad infinitum:)

Easy enough to modify, of course, though it makes for ugly code and only deals with inequalities. The order of evaluation problem can occur anywhere where two random calls are used within the same expression.


All times are GMT -4. The time now is 04:31 AM.

Powered by vBulletin® Version 3.8.1
Copyright ©2000 - 2025, Jelsoft Enterprises Ltd.
Copyright ©1999 - 2025, Shrapnel Games, Inc. - All Rights Reserved.