-
Notifications
You must be signed in to change notification settings - Fork 36
Rolling horizon tutorial #1387
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
Rolling horizon tutorial #1387
Conversation
🤖 CompareMPS report✅ MPS files match |
9e35443 to
2e0b250
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1387 +/- ##
==========================================
- Coverage 98.61% 98.47% -0.14%
==========================================
Files 38 42 +4
Lines 1368 1511 +143
==========================================
+ Hits 1349 1488 +139
- Misses 19 23 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
052a4d1 to
99f0185
Compare
|
📝 Check the documentation preview: https://tulipaenergy.github.io/TulipaEnergyModel.jl/previews/PR1387 |
99f0185 to
f3f8bf7
Compare
|
@clizbe, the tutorial here can be reviewed in parallel to the rest of the rolling horizon changes |
bdbab55 to
f7c284d
Compare
|
This has been rebased and marked as ready to review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @abelsiqueira, I left a minor comments, but generally speaking, I like it 😄
Anyway, let's wait for @clizbe's comments, since she has a sharp eye for the documentation 😉
| Finally, as a sanity check, we can compare that indeed both solutions reach the same demand value by comparing the aggregated outgoing flow and the charge between the rolling horizon and the no-rolling horizon versions. | ||
|
|
||
| ```@example rolling_horizon | ||
| outgoing_rh = thermal_rh + solar_rh + discharge_rh | ||
| outgoing_no_rh = thermal_no_rh + solar_no_rh + discharge_no_rh | ||
|
|
||
| Plots.plot( | ||
| Plots.plot(timestep, outgoing_no_rh - outgoing_rh, title="thermal + solar + discharge error"), | ||
| Plots.plot(timestep, charge_no_rh - charge_rh, title="charge error"), | ||
| layout = (2, 1), | ||
| size = (800, 150 * 2), | ||
| xticks = 1:move_forward:horizon_length, | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the sanity check, it is better to have a table here with the min, max, and average error. In addition, the definition of the error should be only one as:
error = demand + charging - thermal - solar - discharge
The sanity check is that this should be zero or within the tolerances of the solver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Looks like a nice tutorial. I didn't run the code, but read through it. I think there's some context missing (or maybe in different sections of the docs) and I wonder about the window naming. Is this the standard naming convention? If not, can we come up with something more intuitive?
|
Thanks for the review @clizbe, I've committed the easy and commented on the rest |
|
@clizbe, it is not on you, I am the one sorry here, I have not been able to explain myself correctly with the RH, let me try again 😜
|
|
|
||
| Plots.plot(both_plots..., layout = (2, 1), size = (800, 150 * 2)) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this, I would add the comparison of the duals for the sake of completeness.
|
You should make a TED talk! ;) |
8362f38 to
f2cf145
Compare
Co-authored-by: Lauren Clisby <lclisby@gmail.com>
Co-authored-by: Abel Soares Siqueira <abel.siqueira@esciencecenter.nl>
Co-authored-by: Abel Soares Siqueira <abel.siqueira@esciencecenter.nl>
Co-authored-by: Abel Soares Siqueira <abel.siqueira@esciencecenter.nl>
Co-authored-by: Abel Soares Siqueira <abel.siqueira@esciencecenter.nl>
Co-authored-by: Lauren Clisby <lclisby@gmail.com>
f2cf145 to
e338235
Compare
|
@datejada, I've updated the errors now as well, let me know if that's what you're looking for |
datejada
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abelsiqueira Thanks for the changes in the PR. I have a suggestion for the wording in the last part, but I'm pre-approving, so you can merge after accepting the change.
Co-authored-by: Diego Alejandro Tejada Arango <12887482+datejada@users.noreply.github.com>
BLOCKED until other rolling horizon PRs are merged.Create a tutorial for rolling horizon with a similar output to the JuMP tutorial https://jump.dev/JuMP.jl/stable/tutorials/algorithms/rolling_horizon/.
Related issues
Closes #1385
Part of #1365
Checklist