informatics-lab / precip_rediagnosis

Project to use ML to re-diagnose precipitation fields from ensemble model fields
0 stars 0 forks source link

Data extract and prepare refactor #39

Closed stevehadd closed 2 years ago

stevehadd commented 2 years ago

Overall aims of this PR

closes #3 closes #30 closes #31 closes #33 closes #35 closes #17

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

stevehadd commented 2 years ago

This is till in progess, so don't merge yet! Things that are still outstanding:

stevehadd commented 2 years ago

Things you can look at so far:

hannahbrown7 commented 2 years ago

Line 151 in drivers.py - potentially a typo or end of line missing?

hannahbrown7 commented 2 years ago

Tried replacing 4 nested for loops to calculate radar fractions with itertools but unfortunately did not make the function any more efficient. May be some grouping functionality in Xarray that could be helpful to speed this part up.

hannahbrown7 commented 2 years ago

The first iteration in the radar fraction calculation appears to have a large radar_cell_in_mg value of 786898, which doesn't seem right, the rest are somewhere in the region of 420, or slightly less (presumable where radar_cell_in_mg is very small or zero, this is where model cells are outside of the radar domain?)

stevehadd commented 2 years ago

Still to do:

hannahbrown7 commented 2 years ago

I think there may be different units for model and radar data, MOGREPS-G precip is in m and radar precip is in mm. Could this possibly be added in the preprocessing step? MOGREPS_G precip data just needs to be divided by 0.001

hannahbrown7 commented 2 years ago

Not sure if the NaN values are just a problem based on how I'm converting to xarray for plotting the data, but popping this here in case useful. It seems that there are some timestamps each day at 00Z that loose the hour and small number of points that keep it, it would be good to make sure this is consistently including the time as well as date image

stevehadd commented 2 years ago

Right, at last I think this is ready for review. (I haven't currently implemented Pete's suggestion above, I'll do that while people are reviewing. Things I'm looking for from this review:

stevehadd commented 2 years ago

Before I find anything else to fix, I'm pulling the trigger and merging. I'll leave any further problems for susequent issues/PRs.