openclimatefix / india-forecast-app

Runs wind and PV forecasts for India and saves to database
MIT License
1 stars 4 forks source link

Integrating Windnet #12

Closed confusedmatrix closed 6 months ago

confusedmatrix commented 7 months ago

Pull Request

Description

Uses Windnet for producing forecasts when asset_type == "wind"

TODO

Fixes #

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?

Checklist:

peterdudfield commented 6 months ago

Sorry @confusedmatrix Ive done a few updates, hopefully they are not too bad to merge. Let me know if you need any help

confusedmatrix commented 6 months ago

Mostly ready to review - I think the Dockerfile just needs some updating to get it to run the app.

Currently getting this error when trying to run the container:

Rioxarray is not installed, so not importing OpenTopography
Traceback (most recent call last):
  File "/venv/bin/app", line 5, in <module>
    from india_forecast_app.app import app
  File "/venv/lib/python3.11/site-packages/india_forecast_app/app.py", line 18, in <module>
    from .models import DummyModel, PVNetModel
  File "/venv/lib/python3.11/site-packages/india_forecast_app/models/__init__.py", line 4, in <module>
    from .pvnet.model import PVNetModel
  File "/venv/lib/python3.11/site-packages/india_forecast_app/models/pvnet/model.py", line 16, in <module>
    from ocf_datapipes.training.pvnet import construct_sliced_data_pipeline as pv_base_pipeline
  File "/venv/lib/python3.11/site-packages/ocf_datapipes/training/pvnet.py", line 10, in <module>
    from ocf_datapipes.training.common import (
  File "/venv/lib/python3.11/site-packages/ocf_datapipes/training/common.py", line 13, in <module>
    from ocf_datapipes.load import (
ImportError: cannot import name 'OpenTopography' from 'ocf_datapipes.load' (/venv/lib/python3.11/site-packages/ocf_datapipes/load/__init__.py)

Also there is a bug in the tests if they are run between the start of the hour and quarter past - something to do with the init_times in the NWP data:

 KeyError: "not all values found in index 'step'\nThis exception is thrown by __iter__ of SelectTimeSliceNWPIterDataPipe(dropout_frac=0, dropout_timedeltas=None, forecast_duration=datetime.timedelta(days=2), history_duration=datetime.timedelta(seconds=3600), sample_period_duration=datetime.timedelta(seconds=3600), source_datapipe=AddT0IdxAndSamplePeriodDurationIterDataPipe, t0_datapipe=_ChildDataPipe)"
peterdudfield commented 6 months ago

Mostly ready to review - I think the Dockerfile just needs some updating to get it to run the app.

Currently getting this error when trying to run the container:

Rioxarray is not installed, so not importing OpenTopography
Traceback (most recent call last):
  File "/venv/bin/app", line 5, in <module>
    from india_forecast_app.app import app
  File "/venv/lib/python3.11/site-packages/india_forecast_app/app.py", line 18, in <module>
    from .models import DummyModel, PVNetModel
  File "/venv/lib/python3.11/site-packages/india_forecast_app/models/__init__.py", line 4, in <module>
    from .pvnet.model import PVNetModel
  File "/venv/lib/python3.11/site-packages/india_forecast_app/models/pvnet/model.py", line 16, in <module>
    from ocf_datapipes.training.pvnet import construct_sliced_data_pipeline as pv_base_pipeline
  File "/venv/lib/python3.11/site-packages/ocf_datapipes/training/pvnet.py", line 10, in <module>
    from ocf_datapipes.training.common import (
  File "/venv/lib/python3.11/site-packages/ocf_datapipes/training/common.py", line 13, in <module>
    from ocf_datapipes.load import (
ImportError: cannot import name 'OpenTopography' from 'ocf_datapipes.load' (/venv/lib/python3.11/site-packages/ocf_datapipes/load/__init__.py)

Also there is a bug in the tests if they are run between the start of the hour and quarter past - something to do with the init_times in the NWP data:

 KeyError: "not all values found in index 'step'\nThis exception is thrown by __iter__ of SelectTimeSliceNWPIterDataPipe(dropout_frac=0, dropout_timedeltas=None, forecast_duration=datetime.timedelta(days=2), history_duration=datetime.timedelta(seconds=3600), sample_period_duration=datetime.timedelta(seconds=3600), source_datapipe=AddT0IdxAndSamplePeriodDurationIterDataPipe, t0_datapipe=_ChildDataPipe)"

Might need to add https://pypi.org/project/rioxarray/ to requirements to get bug working with docker file. Maybe we should run the tests inside docker, in order to catch these erros in the CI. https://github.com/openclimatefix/ocf_datapipes/blob/main/ocf_datapipes/load/__init__.py - this gave me a clue

Ill have a look at the 0-15 test bug, a bit later

peterdudfield commented 6 months ago

Also could use @freeze_time("2021-01-01 12:00:01") in order to help debugin that first 15 mins bug, although with fixtures im not sure this works

peterdudfield commented 6 months ago

For the first 15 minute bug, lets conside these two examples

IN PROGRESS 9.07 9.25 11.55 12.01 12.17
app.py - timestamp 9.00 9.15 11.45 12.00 12.15
pvnet/utils.py t0_datetime_utc 09.00 09.00 09.00 12.00 12.00
select_time_slice_nwp.py init_time 09.00 12.00 12.00
select_time_slice_nwp.py target_times 11.00 onwards 11.00 onwards 12.00 onwards

Ive added a minus 1 hour to the t0_datetime_utc in the pvnet/utils, which i think will sort this.

peterdudfield commented 6 months ago

15 minute bug is sovled

peterdudfield commented 6 months ago

The docker issue should now be sovled, but im struggling to test it, Ill keep trying.

Would you be able to add the command you use into the readme.md of how to run things locally in the docker file

confusedmatrix commented 6 months ago

The docker issue should now be sovled, but im struggling to test it, Ill keep trying.

Would you be able to add the command you use into the readme.md of how to run things locally in the docker file

Done

peterdudfield commented 6 months ago

Update: Weather data is now there, im tempted to merge this, get this up and running. And solve any bugs. And then @confusedmatrix if you have time get the it working with weather data, I might have to give you AWS access, or we think of another way

peterdudfield commented 6 months ago

I thought I would merge this @confusedmatrix so I can try testing it. Then any bug fixes, or NWP data, can be done on a new branch