switch-model / switch

A Modern Platform for Planning High-Renewable Power Systems
http://switch-model.org/
Other
130 stars 85 forks source link

Potential severe bug in storage module #136

Closed staadecker closed 3 years ago

staadecker commented 3 years ago

The following lines of code seem to be wrong. With the current implementation, StorageEnergyInstallCosts is the same every period.

https://github.com/switch-model/switch/blob/a68b6379f5ea537775b29e5ec7f3f76c091b70b8/switch_model/generators/extensions/storage.py#L136-L143

I think the correct expression would be similar to how GenCapitalCosts is implemented where only build years for the period are included in the sum.

https://github.com/switch-model/switch/blob/a68b6379f5ea537775b29e5ec7f3f76c091b70b8/switch_model/generators/core/build.py#L467-L471

For example if we consider a 20 year project, currently the overnight cost is annualized over 20 years but included in every period (possible a lot more than 20 years).

mfripp commented 3 years ago

Thanks for bringing this up. That bug is fixed in version 2.0.6, but we've been delayed in releasing that version. I'll see if I can publish 2.0.6 in the next few days. Meanwhile, you can replace those lines with this to fix the bug:

    mod.StorageEnergyCapitalCost = Expression(
        mod.STORAGE_GENS, mod.PERIODS,
        rule=lambda m, g, p: sum(
            m.BuildStorageEnergy[g, bld_yr]
            * m.gen_storage_energy_overnight_cost[g, bld_yr]
            * crf(m.interest_rate, m.gen_max_age[g])
            for bld_yr in m.BLD_YRS_FOR_GEN_PERIOD[g, p]
        )
    )
    mod.StorageEnergyFixedCost = Expression(
        mod.PERIODS,
        rule=lambda m, p: sum(
            m.StorageEnergyCapitalCost[g, p] for g in m.STORAGE_GENS
        )
    )
    mod.Cost_Components_Per_Period.append('StorageEnergyFixedCost')

If you'd rather not edit the Switch source code directly, you can put the following code in a "storage_patch.py" file in your working directory and add storage_patch to the module list somewhere after switch_model.generators.extensions.storage.

from pyomo.environ import *
from switch_model.financials import capital_recovery_factor as crf

def define_components(mod):
    del mod.StorageEnergyInstallCosts
    mod.StorageEnergyCapitalCost = Expression(
        mod.STORAGE_GENS, mod.PERIODS,
        rule=lambda m, g, p: sum(
            m.BuildStorageEnergy[g, bld_yr]
            * m.gen_storage_energy_overnight_cost[g, bld_yr]
            * crf(m.interest_rate, m.gen_max_age[g])
            for bld_yr in m.BLD_YRS_FOR_GEN_PERIOD[g, p]
        )
    )
    mod.StorageEnergyInstallCosts = Expression(
        mod.PERIODS,
        rule=lambda m, p: sum(
            m.StorageEnergyCapitalCost[g, p] for g in m.STORAGE_GENS
        )
    )
staadecker commented 3 years ago

Thank you @mfripp! We ended up doing the exact same fix you proposed which is reassuring. If there are other bugs in 2.0.5 that we should be aware of let me know!

When I have more time we should also connect. I'm working with @PatyHidalgo on the RAEL repo and will be working on modules / features that might be useful for other SWITCH teams. If you're interested in including them in this repo, I'd be happy to write some pull requests.

mfripp commented 3 years ago

I've published version 2.0.6 as a bug fix for this, so I'm closing the issue now. Version 2.0.6 is on pypi now and should be on conda-forge in an hour or two. Let me know if you have any problems with the new version.

mfripp commented 3 years ago

I forgot to comment — I'd be more than happy to have pull requests for new features and fixes. We'll need to figure out a process for that. I have a backlog of suggestions from @josiahjohnston, so I really need to do a better job of reviewing/incorporating those.

staadecker commented 3 years ago

Sounds good!