aditya-grover / climate-learn

Source code for ClimateLearn
MIT License
310 stars 50 forks source link

Table 3 (Downscaling experiments results) reports `RMSE` and not `LatWeightedRMSE` #110

Closed patel-zeel closed 1 year ago

patel-zeel commented 1 year ago

Describe the bug Table 3 in Section 4.2 of the paper "ClimateLearn: Benchmarking Machine Learning for Weather and Climate Modeling" reports RMSE but in Appendix B.4.3 (Climate downscaling metrics) it points to Latitude Weighted RMSE (Eq. 2 in Appendix). I ran the code locally and confirmed that the numbers reported in the paper are showing RMSE and not Latitude Weighted RMSE.

Also, I have a question: Why lat_mse is used for training the forecasting module but mse is used for training the downscaling module?

Snapshots

Table 3 from the paper

image

Code snippet

The following snippet shows that load_forecasting_module uses lat_rmse as test_loss but load_downscaling_module uses rmse.

https://github.com/aditya-grover/climate-learn/blob/1a46b08e75ded7dde2d41867d01cfbe5d0d68c2c/src/climate_learn/utils/loaders.py#L215-L246

jasonjewik commented 1 year ago

Hi @patel-zeel, thank you for bringing this to our attention. We used RMSE in accordance with prior works (e.g., https://arxiv.org/pdf/1703.03126.pdf). Since we use plain RMSE as evaluation, we use plain MSE as the optimization objective.

patel-zeel commented 1 year ago

Hi @jasonjewik, thanks a lot for the clarification. It may be helpful to mention this in Appendix B.4.3. It may also help to rename Latitude weighted RMSE as LRMSE or some other acronym to distinguish it from vanilla RMSE. Since I was calculating LRMSE for my models and comparing it directly with Table 3, it made me happy temporarily till I realized this 😅.

jasonjewik commented 1 year ago

Sure thing. I'll coordinate with my co-authors on amending the paper.