Update capacity.jl to use the new attach functions and update tables #963
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #963 +/- ##
==========================================
- Coverage 95.45% 95.27% -0.18%
==========================================
Files 29 29
Lines 1034 1059 +25
==========================================
+ Hits 987 1009 +22
- Misses 47 50 +3 ☔ View full report in Codecov by Sentry. |
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
abelsiqueira
force-pushed
the
642-capacity
branch
from
568834e to
7234e80
Compare
abelsiqueira
changed the title
abelsiqueira
force-pushed
the
642-capacity
branch
from
7234e80 to
0701193
Compare
|
This is not the whole of #961, but it's already a lot of changes - because it's one of the hardest files, I think. @datejada, what do you think? Does it look clean enough? |
There was a problem hiding this comment.
@abelsiqueira I have committed my proposed changes. Let me know if you agree on them to continue. Thanks!
| ] | ||
| profile_times_capacity = Dict( | ||
| key => begin | ||
| table_name = Symbol("ramping_$(key)_unit_commitment") |
There was a problem hiding this comment.
Also, it reads better below if the keys are :with_unit_commitment and :without_unit_commitment
So, please change it either here or in the other PR you are working for the ramping. Thanks!
There was a problem hiding this comment.
I agree. I will change in the next PR, because I had to completely change the keys to the table names, since they are used in other tables as well
src/constraints/create.jl
Outdated
| @@ -149,7 +171,7 @@ function _create_constraints_tables(connection) | |||
| DuckDB.query( | |||
| connection, | |||
| "CREATE OR REPLACE TEMP SEQUENCE id START 1; | |||
| CREATE OR REPLACE TABLE cons_capacity_outgoing_binary AS | |||
| CREATE OR REPLACE TABLE cons_capacity_outgoing_storage_with_binary AS | |||
There was a problem hiding this comment.
Took me a while, but if I understood correctly, this table should have a WHERE condition to check NOT asset.investable for it to be equal to the old code.
My logic (and I will use initials to avoid writing for 30 minutes) is that initially you had two tables: COB and COBI
- For COB, if investable, use
(c * aiu + ail) * (1 - ch)else usec * aiu * (1 - ch) - FOR COBI, use
c * (aiu * (1 - ch) + aiusm)
Now you have three tables, COSB, COISB, COISLB.
- For COSB,
c * aiu * (1 - ch), so COSB = COB + not investable - For COISB,
c * (aiu * (1 - ch) + aiusm), so COISB = COBI - For COISLB,
(c * aiu + ail) * (1 - ch), so COISLB is COB + investable (which is COBI, I think)
If that is the case, you can also reuse the COISB table - you still separate the constraints to avoid the if condition, you just attach to the same constraint.
I checked the formulation, and indeed it separates like these tables, but I think that the formulation is also missing a a \not \in A_y^I or similar for that constraint.
Let me know if this makes sense or not
There was a problem hiding this comment.
It makes sense, I left a couple of comments where I see we need the changes and also a rename in one of the constraints to make it explicit.
There was a problem hiding this comment.
We need to update the formulation, too
There was a problem hiding this comment.
Thanks. Do you want to finish the PR with these changes?
Part of #961