meteofrance / py4cast

Weather forecasting with Deep Learning
10 stars 10 forks source link

Refacto py4cast start #102

Open clemoule opened 2 weeks ago

clemoule commented 2 weeks ago

implementation of :

A faire:

(certain)

(peut etre)

pas encore de autoencoderDataModule donc pour l'instant comportement encoder-decoder (x!=y) et pas autoencoder(x=y).

cbovalo commented 2 weeks ago

@clemoule Is this PR ready for review or is it a draft ? BTW, after a quick review, I find the code would be more readable.

clemoule commented 2 weeks ago

@clemoule Is this PR ready for review or is it a draft ? BTW, after a quick review, I find the code would be more readable

The code can be executed but i need to verify the plot/log method. We will discuss of the way to redesign the trainers, parsers, lightningmodule and datamodule this afternoon. So this is more of a draft even though it works. We will be set this afternoon.

clemoule commented 2 weeks ago

storing functions in classes (ex: PlotLightningModule) is a bit of over engineering but it helps to group functions and methods by purpose. It makes the code easier to map and understand => quicker to pick up the code => quicker to implement additions to the code and quicker to tinker

cbovalo commented 2 weeks ago

I think it's also a good thing to decouple computing metrics and making plots. I'll make some comments on the code if improvement is needed.