Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Battery storage cost constant version 2 #348

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

toddleif
Copy link
Collaborator

@toddleif toddleif commented Feb 6, 2024

This new code allows for a battery storage cost constant to be included in the model. This feature was added in order to align REopt's battery cost modeling with NREL's Annual Technology Baseline (ATB). Changes to the code include:

  • In the src/core/energy_storage/electric_storage.jl struct, the following fields were added: installed_cost_constant, replace_cost_constant, and cost_constant_replacement_year. All three of the added fields default to zero.
  • The new binary variable binIncludeStorageCostConstant was added. This variable is indexed on p.s.storage.types.elec.

@toddleif toddleif requested a review from adfarth February 6, 2024 01:28
@adfarth
Copy link
Collaborator

adfarth commented Feb 6, 2024

@toddleif could you add the new inputs to this text in electric_storage.jl, starting line 33:

When modeling degradation the following ElectricStorage inputs are not used:
    - `replace_cost_per_kw`
    - `replace_cost_per_kwh`
    - `inverter_replacement_year`
    - `battery_replacement_year`

src/results/financial.jl Outdated Show resolved Hide resolved
src/core/reopt.jl Outdated Show resolved Hide resolved
src/core/reopt.jl Outdated Show resolved Hide resolved
@adfarth
Copy link
Collaborator

adfarth commented Feb 6, 2024

@toddleif in general this is looking good! In addition to my in-line comments above, just a few high-level notes:

  • Can you add tests to test/test_with_xpress.jl and runtests.jl that check that things are working as expected with your new addition? This could check, for example, some expected financial outputs, that you have manually calculated in Excel.
  • I think we need to reconfigure the constraint to not be an indicator constraint, if possible. I suspect this is why so many of the hightests are failing. We'll want all tests to be passing before we merge.

@toddleif toddleif marked this pull request as ready for review July 3, 2024 17:24
@adfarth
Copy link
Collaborator

adfarth commented Jul 3, 2024

@toddleif could you please update the CHANGELOG.md file with a description of these changes? You can see other entries in there for examples. For this PR, you could list all 3 new inputs that are added and note that they default to zero. After you add that, you could update the PR description with the same information (should be a bit more detailed than what is currently the PR description). The PR description could also include a bit of reasoning for why we made this change (e.g. reference the ATB cost structure)

src/core/reopt.jl Outdated Show resolved Hide resolved
src/core/reopt.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
Updates to the method of applying the ElectricStorage cost constant constraints, the binary variable name for including the cost constants, and the tests for the ElectricStorage cost constant
@toddleif toddleif requested a review from adfarth November 4, 2024 01:04
@@ -169,10 +172,13 @@ end
can_grid_charge::Bool = off_grid_flag ? false : true
installed_cost_per_kw::Real = 910.0
installed_cost_per_kwh::Real = 455.0
installed_cost_constant::Real = 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toddleif could you please add a brief description of this input to this markdown area?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea. I just added a description.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toddleif I just modified the description to be in-line with the inputs and to just give a short high-level description of what that input is. Could you review?

src/results/financial.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
test/test_with_xpress.jl Outdated Show resolved Hide resolved
@adfarth
Copy link
Collaborator

adfarth commented Nov 4, 2024

@toddleif I think this is looking good! I left a few comments throughout, and have noticed just two more places where changes may need to be made:

  • It looks like you'll need to make a few changes to proforma.jl. See e.g. line 71 "# calculate Storage o+m costs, incentives, and depreciation" (there may be other places in there that require updates too)
  • outage_constraints.jl line 80 where I think the cost constant should be added to the "bigM" value being subtracted from the capital costs
@constraint(m,
            m[:dvMGStorageUpgradeCost] >= p.s.financial.microgrid_upgrade_cost_fraction * m[:TotalStorageCapCosts] - (
                p.s.financial.microgrid_upgrade_cost_fraction * p.third_party_factor * (
                    sum( p.s.storage.attr[b].net_present_cost_per_kw * p.s.storage.attr[b].max_kw for b in p.s.storage.types.elec) + 
                    sum( p.s.storage.attr[b].net_present_cost_per_kwh * p.s.storage.attr[b].max_kwh for b in p.s.storage.types.all )
                ) * (1-m[:binMGStorageUsed])  # Big-M is capital cost of battery with max size kw and kwh
            )
        )

@adfarth
Copy link
Collaborator

adfarth commented Nov 4, 2024

Also noting that the tests did fail previously (looks like they may have timed out) so I re-ran them on Tucker's last commit. The commit that I made was just a change to the Changelog, so only the documentation tests passed, which is misleading :)

@toddleif
Copy link
Collaborator Author

toddleif commented Nov 7, 2024

@toddleif I think this is looking good! I left a few comments throughout, and have noticed just two more places where changes may need to be made:

  • It looks like you'll need to make a few changes to proforma.jl. See e.g. line 71 "# calculate Storage o+m costs, incentives, and depreciation" (there may be other places in there that require updates too)
  • outage_constraints.jl line 80 where I think the cost constant should be added to the "bigM" value being subtracted from the capital costs
@constraint(m,
            m[:dvMGStorageUpgradeCost] >= p.s.financial.microgrid_upgrade_cost_fraction * m[:TotalStorageCapCosts] - (
                p.s.financial.microgrid_upgrade_cost_fraction * p.third_party_factor * (
                    sum( p.s.storage.attr[b].net_present_cost_per_kw * p.s.storage.attr[b].max_kw for b in p.s.storage.types.elec) + 
                    sum( p.s.storage.attr[b].net_present_cost_per_kwh * p.s.storage.attr[b].max_kwh for b in p.s.storage.types.all )
                ) * (1-m[:binMGStorageUsed])  # Big-M is capital cost of battery with max size kw and kwh
            )
        )

@adfarth I addressed these edits too, thanks for catching these as well!

@@ -128,6 +128,16 @@ function add_elec_storage_dispatch_constraints(m, p, b; _n="")
end
end

function add_elec_storage_cost_constant_constraints(m, p, b; _n="")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toddleif If I'm understanding correctly, I think the following could work for avoiding the increased run time even when a cost constant isn't included:

  • Move the if statementif p.s.storage.attr[b].installed_cost_constant != 0 || p.s.storage.attr[b].replace_cost_constant != 0 to reopt.jl. Within that statement, add a warning about binary vars and then call "add_elec_storage_cost_constant_constraints".
  • Define the binary vars within "add_elec_storage_cost_constant_constraints" here. (see add_prod_incent_vars_and_constraints as an example)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants