Add min flow limit without unit commitment constraint#1614
Conversation
🤖 CompareMPS report✅ MPS files match |
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
|
📝 Check the documentation preview: https://tulipaenergy.github.io/TulipaEnergyModel.jl/previews/PR1614 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1614 +/- ##
==========================================
+ Coverage 98.60% 98.61% +0.01%
==========================================
Files 55 56 +1
Lines 1862 1880 +18
==========================================
+ Hits 1836 1854 +18
Misses 26 26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@abelsiqueira Sorry to bother you. Since Ni is on paternity leave, would you mind having a look at this PR? It is a simple one, since it adds constraints; it doesn't change any of the current logic or structure. The benchmark shows basically no impact in the EU case study, which is good and expected (since there are no constraints like this there). Of course, there is always the question of how these new constraints scale, but since this one doesn't change any current logic and follows the same pattern as other constraints, it should be fine. Until we take benchmarking all the different constraints more seriously (which I think should be the way). Anyway, please let me know your comments, and if you don't have time for this, it is okay. I will see what we can do internally. As always, the changes read better by commit. Thanks! Diego |
abelsiqueira
left a comment
There was a problem hiding this comment.
Hi @datejada, sorry for the delay on answering, and thanks for pinging on slack, otherwise I'd miss it.
I took a quick look.
The constraints tables and objects seems to be correct. I haven't checked the mathematical formulation in the docs or the issue to compare to what is implemented, since it's beyond my expertise.
I took a quick look at the tests, and they seem to capture the relevant main cases. I haven't evaluated corner cases.
The benchmark speed changed are well within our expected variation. Since you mentioned that EU doesn't have these constraints, I think the memory changes are due to either just creating small (order of Kb) variables, or type instabilities that we still can't avoid.
Let me know if this review is enough.
I made small suggestions that don't affect correctness, so it's approved (with the caveats above)
Co-authored-by: abelsiqueira <1068752+abelsiqueira@users.noreply.github.com>
|
@abelsiqueira, thanks for the comments and suggestions. I have included them in the latest commit in this PR. |
The implementation includes:
cons_min_output_flow_without_unit_commitment_*tables in create-constraints.sql@timeitcall added in create-model.jlFor the test, different edge cases are included:
Related issues
Closes #1597
Checklist