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
Mix in timely beliefs Sensor #13
Conversation
Flix6x
commented
Feb 1, 2021
•
edited
edited
- Mix in timely beliefs Sensor.
- Add custom database revision.
Add custom database revision.
Note that I also set the following defaults for sensors:
Where do you think that information should live, documentation wise? |
Two todos refer to the use of knowledge horizons/times: todo: take into account knowledge horizon function todo: interpret belief horizons with respect to knowledge time rather than event end. Do you feel like I should tackle these issues within this PR? For me it feels like they deserve a PR of their own. |
First question: Do our existing tests adequately cover functionality touched here? |
Tldr: deriving a horizon for POSTed data and created forecasts may not be
adequately tested. Existing tests should not be affected, but adding tests
makes sense. I'll add them.
FlexMeasures functionality is still almost completely geared towards the
use of belief horizons rather than belief times: time series are stored
with their belief horizon rather than their belief time, and the UI also
filters by belief horizon rather than by belief time. The few exceptions I
can think of are the API's prior field (which I propose to tackle in a
separate PR), and the derivation of the horizon for POSTed data (without an
explicit horizon) and created forecasts. To my knowledge, we have no
explicit checks for the belief time derived from the belief horizon of a
specific data point. Such tests would have added no value before (testing
for the expected horizon was sufficient), but from this PR onwards, adding
explicit tests for the expected belief time of data makes sense. I'll go
through our existing tests and add said checks.
…On Tue, Feb 2, 2021, 17:33 Nicolas Höning ***@***.***> wrote:
First question: Do our existing tests adequately cover functionality
touched here?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#13 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHJ5BS5ATZYYVI4HQWBS3FDS5ASMTANCNFSM4W5EVBGQ>
.
|
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.
Looks like a very nice PR to me.
I have a few questions, and I am responding to your question about design of func store identifiers.
...s/versions/22ce09690d23_mix_in_timely_beliefs_sensor_with_asset_market_and_weather_sensor.py
Show resolved
Hide resolved
That part of my suggestion would mean to store the number in the
database (not the function name). So even if we change the name of the
constant, we won;t have to update the database.
Of course, if we truly treat is as a constant, we won't change it. And legibility is of course good. I wasn't worrying about database size.
|
More todo items linked to this issue: Todo: until we sorted out the ex_post_horizon, use all available price data Todo: replace 0 hours with whatever the moment of switching from ex-ante to ex-post is for this generic asset |
@nhoening I propose to limit the scope of this PR to the introduction of the TB mixin, and to deal with the paradigm shift of knowledge horizon functions in a separate PR, because this paradigm shift touches many parts of our code and I'd like to prevent unnecessarily large PRs. To ensure forecasting and scheduling functionality is not affected, we could choose to set the knowledge horizon function of day-ahead markets to 0h ex_post for the time being, which represents the previous state of affairs. Tariffs, power sensors and weather sensors would already be properly moved to the new state of affairs. The latter two will not actually "feel" the change, while the tariff data will become much simpler and can still be used for scheduling. |
Alright, sounds good. I trust you with the tariff data here. |