search

georgemilosh / Climate-Learning

How to predict extreme events in climate using rare event algorithms and modern tools of machine learning
https://georgemilosh.github.io/Climate-Learning/
MIT License
20 stars 5 forks source link

The ability to work with CESM data using ERA_Fields_New.py and Learn2… #44

Closed georgemilosh closed 1 year ago

georgemilosh commented 1 year ago

Handling of data a bit different given specificities of CESM Small fixes in tutorial.ipynb

georgemilosh commented 1 year ago

@AlessandroLovo , it allows me to merge, but I think it would be a good idea if you have a look. I am modifying some core functionality.

AlessandroLovo commented 1 year ago

I wrote some comments on your changes. The one I am most concerned about is the manipulation of the latitude. Have you checked that it doesn't break Plasim data?

georgemilosh commented 1 year ago

I am running some tests. Like making plots and some CNN training. The reason I have this sorting of latitudes is because Plasim and CESM have opposite ordering. So I put something to enforce the sorting consistent with Plasim. With that being said, as you indicate, it would be desirable to sort the latitudes in default case as well.

Concerning the point about using Model to decide where the data comes from, this would be ok, but current implementation gives us more flexbility where data could come from.

georgemilosh commented 1 year ago

So I decided to move the sorting into a separate Plasim_field class method which is called in Learn2_new.py: load_data()

georgemilosh commented 1 year ago

Ok, on my side the tests look good. I think it would be nice to merge pull request soon

georgemilosh commented 1 year ago

Also the changes you proposed seem to work, @AlessandroLovo

georgemilosh commented 1 year ago

Actually there is one more point that is worth doing. I think it would be better for fields_infos to be loaded from a json file, like we do with config.json. This is because I would like to keep the labels of the fields, while working with both CESM and Plasim

AlessandroLovo commented 1 year ago

Maybe make a separate issue for that?