noaa-ocs-modeling / EnsemblePerturbation

perturbation of coupled model input over a space of input variables
https://ensembleperturbation.readthedocs.io
Creative Commons Zero v1.0 Universal
7 stars 3 forks source link

Some CORR coefficients in the output of `plot_kl_surrogate_fit()` function are not correct. #140

Closed FariborzDaneshvar-NOAA closed 5 months ago

FariborzDaneshvar-NOAA commented 5 months ago

Here is an example output of plot_kl_surrogate_fit() function (from ensembleperturbation.plotting.surrogate)

image

The last four modes should have CORR of 0.0 instead of 1.0!

Looks like there is an issue with the get_validation_statistics()

FariborzDaneshvar-NOAA commented 5 months ago

@SorooshMani-NOAA @WPringle I found the bug. Please see line 30 in surrogate.py .

if numpy.isnan(corr.values):
    corr.values = 1.0  # 0/0: perfect correlation
return corr

Replacing 1.0 with 0.0 will resolve this error, but it might not be correct all the times (i.e. when both obs. and sim. are zero). How about returning NaN or Not Available instead of a number (1.0 or 0.0)?

SorooshMani-NOAA commented 5 months ago

Can't we just add another condition when it's nan to distinguish perfect vs no-relation?

FariborzDaneshvar-NOAA commented 5 months ago

@SorooshMani-NOAA good point. how about something like this to differentiate perfect vs non-relation:

if (PDOD==0) & (PD2==0) & (OD2==0):
    corr.values = 1.0  # 0/0: perfect correlation
elif (PD2==0) or (OD2==0):
    corr.values = 0.0  # No./0: no-correlation
SorooshMani-NOAA commented 5 months ago

I think it makes sense, but wait for @WPringle to confirm if you're not sure

WPringle commented 5 months ago

I think it looks good, as long as result gives you what we think it should be.

FariborzDaneshvar-NOAA commented 5 months ago

@SorooshMani-NOAA I'm getting permission denied for pushing my changes to the remote (git push origin fix_cor_in_plot):

ERROR: Permission to noaa-ocs-modeling/EnsemblePerturbation.git denied to FariborzDaneshvar-NOAA.
fatal: Could not read from remote repository. 

Please make sure you have the correct access rights and the repository exists. 

I pushed changes from the same directory on NHC_COLAB_2 to the remote before! .git/config file also has pushurl as you noted before. What's your take on this? thanks

WPringle commented 5 months ago

@FariborzDaneshvar-NOAA I also could not push to remote. You need just push to your fork and then to get into the main repository you need to do a pull request.

SorooshMani-NOAA commented 5 months ago

@FariborzDaneshvar-NOAA I added you to the repo

FariborzDaneshvar-NOAA commented 5 months ago

PR: https://github.com/noaa-ocs-modeling/EnsemblePerturbation/pull/141