openclimatefix / ocf_datapipes

OCF's DataPipe based dataloader for training and inference

MIT License

13 stars 11 forks source link

add check for non nans in satellite data #210

Closed peterdudfield closed 1 year ago

peterdudfield commented 1 year ago

Pull Request

Description

Have option for PVnet to raise error if there are nans in the satellite data

Helps with https://github.com/openclimatefix/PVNet/issues/31

How Has This Been Tested?

Added a test

[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
[x] I have added tests that prove my fix is effective or that my feature works
[ ] I have checked my code and corrected any misspellings

codecov[bot] commented 1 year ago

Codecov Report

Merging #210 (363c73b) into main (bab6d64) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
+ Coverage   81.31%   81.34%   +0.03%     
==========================================
  Files         128      128              
  Lines        5539     5548       +9     
==========================================
+ Hits         4504     4513       +9     
  Misses       1035     1035

Impacted Files	Coverage Δ
ocf_datapipes/training/pvnet.py	`81.65% <100.00%> (+1.03%)`	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

peterdudfield commented 1 year ago

Looks good.

We might want to com back to this later to allow some NaNs. If the 30/60 minute delay wasn't the cause of the zig-zag then we might want to go back to a model which can use either, and therefore best use the available inputs. But I think this is good now

Yea good point, I think then having the bool to turn it on and off is good