openclimatefix / Open-Source-Quartz-Solar-Forecast

Open Source Solar Site Level Forecast
MIT License
43 stars 40 forks source link

Feature requested by Issue #26 with additions #103

Closed Hapyr closed 2 months ago

Hapyr commented 3 months ago

Pull Request

Description

change data.get_nwp() in a way that the nwp data is available for dates more than 90 days in the past. The historical api from open meteo is used for this purpose. As this does not contain any visibility data, the value is set to the maximum of 24000 km. In addition, the 'openmeteo-requests' pip package is used. this is clearer and uses a binary data format instead of json so that the transmission is faster. And so the string tinkering for the api request can be omitted.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 79.94%. Comparing base (05ae555) to head (cf6f7a7). Report is 4 commits behind head on main.

:exclamation: Current head cf6f7a7 differs from pull request most recent head 3ef6ff2. Consider uploading reports for the commit 3ef6ff2 to get more accurate results

Files Patch % Lines
quartz_solar_forecast/data.py 93.75% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #103 +/- ## ======================================= Coverage 79.94% 79.94% ======================================= Files 12 12 Lines 354 354 ======================================= Hits 283 283 Misses 71 71 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Hapyr commented 3 months ago

26

peterdudfield commented 2 months ago

Hi @Hapyr , thank you so much for doing this. This looks really great.

Just becasue this is a significant and ciritical part of the code, would you mind running the forecast (see example) for a location on 1st April with your code and on the main branch, and just check we get the same forecast.

Hapyr commented 2 months ago

Sorry for the many single commits. I'm a bit confused about the tests that don't work. The tests are currently not running on the main, so I made a PR for the test files: #105

Hapyr commented 2 months ago

@peterdudfield The result on the main branch is not 100% the same as on the new implementation. (However, the result from get_nwp() is exactly the same). Even if I run the model on the main branch twice, the results are slightly different. The differences are only very small and are probably due to the random sorting out of data during processing on the model?!