ANL-CEEESA / UnitCommitment.jl

Optimization package for the Security-Constrained Unit Commitment Problem
Other
104 stars 24 forks source link

Add reserve shortfall penalty #16

Closed iSoron closed 3 years ago

iSoron commented 3 years ago

This is the same PR as #13, but keeping only changes related to reserve shorfall penalty.

@akazachk Could we focus on getting this PR merged first, then come back to the other changes introduced in #13? Please feel free to commit to branch feature/reserve-shortfall directly. Any commits added to that branch will appear here.

akazachk commented 3 years ago
  • [ ] Fix _validate_reserve_and_demand (this PR breaks it)

Working on this next.

Question: for reserves, is the key "Reserve (MW)" or "Spinning (MW)"? Both are used, I think:

https://github.com/ANL-CEEESA/UnitCommitment.jl/blob/07d7e047285d7e3b879c347a844b7f1e85b1721a/src/validation/validate.jl#L48

https://github.com/ANL-CEEESA/UnitCommitment.jl/blob/ea35c3ffcc12546bfea7df6c631778303ac17050/src/instance/read.jl#L204

iSoron commented 3 years ago

for reserves, is the key "Reserve (MW)" or "Spinning (MW)"? Both are used, I think

"Spinning (MW)" is used in the input file and "Reserve (MW)" is used in the output JSON dictionary. It's inconsistent, but please don't worry about this right now. I'm planning to add support for multiple reserves, and I'll fix this inconsistency then.

akazachk commented 3 years ago
  • [ ] Fix _validate_reserve_and_demand (this PR breaks it)

Working on this next.

I think this is now fixed... but check 3d252c5. I accidentally committed before pulling so that triggered the merge. I am not sure how to avoid that (reset --soft?).

for reserves, is the key "Reserve (MW)" or "Spinning (MW)"? Both are used, I think "Spinning (MW)" is used in the input file and "Reserve (MW)" is used in the output JSON dictionary. It's inconsistent, but please don't worry about this right now. I'm planning to add support for multiple reserves, and I'll fix this inconsistency then.

👍

iSoron commented 3 years ago

Thanks, looks good now. I'll merge after all tests finish running.

I accidentally committed before pulling so that triggered the merge. I am not sure how to avoid that (reset --soft?).

No worries. You can avoid that by always running git pull --rebase instead of git pull.

iSoron commented 3 years ago

Looks like some tests are failing:

  Got exception outside of a @test
  type Model has no field obj
  Stacktrace:
    [1] getproperty(x::Model, f::Symbol)
      @ Base ./Base.jl:33
    [2] _add_reserve_eqs!(model::Model)
      @ UnitCommitment ~/work/UnitCommitment.jl/UnitCommitment.jl/src/model/formulations/base/system.jl:48
akazachk commented 3 years ago

Thanks, looks good now. I'll merge after all tests finish running.

I made a few updates last night and now all checks are passing.

I accidentally committed before pulling so that triggered the merge. I am not sure how to avoid that (reset --soft?).

No worries. You can avoid that by always running git pull --rebase instead of git pull.

Thanks. I will set git config --global pull.rebase true from now on. Seems like a good default.

iSoron commented 3 years ago

Merged in 000215e991e78d0cf8978ef272c29b748fbd9762

akazachk commented 3 years ago

@iSoron this says "Closed with unmerged commits" --- is there something left to be merged? if not, we can delete this branch.