added forecast saving option for config, adjusted forecast tests#801
added forecast saving option for config, adjusted forecast tests#801reinecfi wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #801 +/- ##
==========================================
- Coverage 80.79% 80.49% -0.31%
==========================================
Files 56 56
Lines 8670 9047 +377
==========================================
+ Hits 7005 7282 +277
- Misses 1665 1765 +100
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| NOTE: Elastic demands are not supported currently | ||
| """ |
There was a problem hiding this comment.
What do you mean with "are not supported currently"?
Are those demands not included at all or are those demands approximated as inelastic demands? When using it I'd want to know wheter the naive residual load forecast is rather under- or overestimated
There was a problem hiding this comment.
Adjusted the wording here. The forecast will underestimate the residual load if elastic demand is present.
| across all connected lines as the node's congestion signal. | ||
|
|
||
| Returns an empty dict if grid data (buses/lines) is unavailable. | ||
| NOTE: Elastic demands are not supported currently |
There was a problem hiding this comment.
More or less like above, but since some demand is ignored, the forecast should be smaller (which can lead to a smaller or bigger congestion in absolute terms, depending whether the net load is positive or negative).
Not sure if adding a comment on this will be more confusing than helpful. Especially as this is a naive forecast.
| ) | ||
| return {} | ||
|
|
||
| # TODO: Add support for elastic demand (currently error of only elastic demands) |
There was a problem hiding this comment.
Is an error thrown? How would I figure out what's wrong and how to fix it?
There was a problem hiding this comment.
I just forgot to remove the TODO here. The if clause circumvents the problem. Currently you are not notified. Maybe one could/should add a warning here?
mthede
left a comment
There was a problem hiding this comment.
Hi Finn, thanks for the nice feature of saving the forecasts! That's a great way to check whether the intended values were provided to the units.
I was at first confused by the naming and defaults of the config parameters. Then I realized that we don't really have full documentation of the top-level config options, other than in Notebook 02.
Maybe it's worth documenting the behavior at one place and link to the specific forecaster details in the docs?
Other than those clarifications and documentations, it looks good :)
…astic demand handling
Related Issue
Closes #797
Closes #800 (other parts fixed by PR #792)
Description
This change adds the possibility to save forecasts and improves forecaster tests.
Checklist
docfolder updates etc.)