farhanrahman/kyoto

updateGDPRate() is incredibly broken

Closed this issue · 5 comments

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.

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.

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.

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.

I havent mirrored the formula into the opposite quadrant.

Should be fixed by d75e6c3