openclimatefix / nowcasting_datamodel

Datamodel for the nowcasting project

6 stars 6 forks source link

Issue/remove none distinct last 7 days #289

Closed peterdudfield closed 4 months ago

peterdudfield commented 4 months ago

Pull Request

Description

only save distinct forecast_value_last_seven_days
This keeps the table smaller
Some forecast linked with forecast_value_last_seven_days will not show the entire forecast
check how long a save takes, as they is now an extra query, the first time running this, it took 3 seconds for each hour, but It seems to speed up with each time it is run
The table went down from ~18 million to ~5 million

How Has This Been Tested?

Chcekd that the metrics still produce sensible results
CI tests
[x] Yes

Checklist:

[x] My code follows OCF's coding style guidelines
[x] I have performed a self-review of my own code
[ ] I have made corresponding changes to the documentation
[ ] I have added tests that prove my fix is effective or that my feature works
[x] I have checked my code and corrected any misspellings

peterdudfield commented 4 months ago

Worth noting that this might introduce tech debt if some of the metrics depend on the deleted data. For future reference, could be fixed by having the remaining forecast values referencing many forecasts.

Ill add this comment in

peterdudfield commented 4 months ago

@devsjc this functionally could be set to false by default? This might reduce the risk abit?

devsjc commented 4 months ago

Good idea!