farhanrahman / kyoto

18 stars 7 forks source link

updateGDPRate() is incredibly broken #107

Closed n1kunj closed 12 years ago

n1kunj commented 12 years ago

The stripped down (but otherwise identical) version of updateGDPRate() I'm using in my behaviour code is, very, very often returning utterly ridiculous GDP rates.

Example inputs:

oldGDPRate = 0.021889750484631952 energyOutput = 974232.4944951184 prevEnergyOutput = 996503.8463622332

returns a GDP Rate of -0.58 (equivalent to 58% reduction in GDP!)

oldGDPRate = -0.5842203056518727 energyOutput = 974232.4944951184 prevEnergyOutput = 974232.4944951184

returns a new GDP Rate of -103.8367418336472 (!!!!!)

    private static double calculateGDPRate(double oldGDPRate,
            double energyOutput, double prevEnergyOutput) {

        double sum;
        double marketStateFactor = 0.5;
        if (energyOutput - prevEnergyOutput >= 0) {
            sum = (((energyOutput - prevEnergyOutput) / prevEnergyOutput)
                    * GameConst.getEnergyGrowthScaler() * marketStateFactor + oldGDPRate * 100) / 2;
        } else {
            sum = ((((energyOutput - prevEnergyOutput) / prevEnergyOutput) * GameConst
                    .getEnergyGrowthScaler()));
        }
        double GDPRate = (GameConst.getMaxGDPGrowth() - GameConst
                .getMaxGDPGrowth()
                * Math.exp(-sum * GameConst.getGrowthScaler()));

        GDPRate /= 100; // Needs to be a % for rate formula

        return GDPRate;
    }

As you can see, code is a direct copy and paste from AbstractCountry with an assumption made for marketStateFactor.

tumblerer commented 12 years ago

I'll take a look at it tomorrow. If you could come in that would be even better. It should never return values like that since it should be trapped by the exponential function between +/-0.07.

tumblerer commented 12 years ago

As an aside, that's an enormous energy drop. 2.3% in one year? Considering you need to drop around 0.5% per year normally, and you wouldn't normally do this by removing industry solely. Still it should handle this.

I'll admit, I havent tested it with largish values, but the theory should hold I reckon. Maybe I need to adjust how it drops.

n1kunj commented 12 years ago

It's part of the behavior which tests the long term repercussions of shutting down factories just so you can sell off lots of credits. This behavior should end up being balanced out but it still needs to be considered as a viable strategy for my mini simulation to work. Unfortunately because the formula is returning such broken values (if you simulate with a particularly large GDP rate for multiple years you end up getting -INFINITY returned) it's breaking everything else.

Remember how I said that the new GDP formula was massively reducing the number of branches in my simulation? Turns out this was the cause.

tumblerer commented 12 years ago

I havent mirrored the formula into the opposite quadrant.

tumblerer commented 12 years ago

Should be fixed by d75e6c33cfd399a615f0e1aa1ab478ac15ee4fb6