FESOM / fesom2

Multi-resolution ocean general circulation model.
http://fesom.de/
GNU General Public License v3.0
49 stars 48 forks source link

change missval from 0 to nan #289

Open JanStreffing opened 2 years ago

JanStreffing commented 2 years ago

Currently the missval in default fesom2 output where bathymetry cuts off the data is 0. The result is that addressing the wrong section of arrays does not lead to a crash. It seems to me that this removes a safety layer, as one might address values underground without noticing. One such case was: https://github.com/FESOM/fdiag/issues/10#issuecomment-1023701955

I suggest we switch to using nan or something like -9e+33.

koldunovn commented 2 years ago

Although logically it make sense and I hate to have 0 in temperature (and other fields), which is actually a valid value, from practical points of view dealing with nans or very large numbers in netCDF files always cause problems. One has to remember to put zeros to missing values explicitly, but that's easier than dealing with peculiarities of how different software (I am looking at you, cdo) treat nans or very large numbers (which are not well defined if you, say, convert from double to single). So my vote is currently would be against it.

JanStreffing commented 2 years ago

I'm very much against keeping the 0 there. I ran into this issue again at in #323 Having 0 as the missval is such an usual choice, that I'd be surprised this is the last we hear of it. While setting some very large number might not be perfect, it will at least show users right away that something is wrong if they treat is like a physical value. Meanwhile setting 0 will result in everything seemingly correct, while the results are quietly wrong.

chrisdane commented 2 years ago

I agree with Jan.

Nikolay, can you give an example? I never had problems with missval values nan or -9e+33.

pgierz commented 2 years ago

I also would need an example here of using large numbers as NaN being a bad idea. In NetCDF, we have the option of declaring which value shall be viewed as a missing number with the _FillValue attribute: https://docs.unidata.ucar.edu/netcdf-c/current/fill_values.html

Although I share Nikolay's grumbly relationship with CDO (it has bitten me in the butt once to often), this _FillValue appears to be valid in CDO, NCO, and xarray. The other widespread data analysis toolkit is R, however I have no experience there, and do not particularly want to gather any. Maybe @chrisdane can comment about the R world.

pgierz commented 2 years ago

One follow up note: I would not use nan. This is ambiguous in various programming languages. Instead, I would use, as @JanStreffing suggested at the beginning, an absurdly large negative number.

koldunovn commented 2 years ago

I am not using cdo for a long time now, so can't give you concrete example anymore. As far as I remember problems begin to appear, for example, when one do a conversion from float32 to float64 and back - not all software understand what should be done properly, and you end up with trash in missing values that can be anything. At the end of the day I would maybe agree with something like -99999 rather than -9e+33.

Maybe I am too old and things are fixed everywhere, and now all software is working perfectly with very large/small numbers. To me it's a complication that adds another point of failure to the system. @dsidoren @patrickscholz any opiinions?

pgierz commented 2 years ago

now all software is working perfectly

Nikolay if that is the case, I will buy you a beer ;-)

Once we have an answer I can volunteer to look at the actual FORTRAN parts to make it happen.

koldunovn commented 2 years ago

Well, first you have to convince @dsidoren and @patrickscholz :)

JanStreffing commented 2 years ago

An argument for changing:

I just post-processed the model output of 37 different CMIP6 models as well as from FESOM2. Not a single one of these models uses 0 as the missval. Although I know FESOM2 better than any of those other models, in the end I had the most trouble with FESOM2. I think this needs to be changed, otherwise FESOM2 will be likely be dropped from future multi model studies much more than FESOM1 output did.

koldunovn commented 2 years ago

@JanStreffing What you post processed was CMORised data, and they have standards for missing values. From my (quite extensive) experience of working with different original ocean model data, there is a good number of them use 0 as missing value, including, for example, ICON.

Making things better for the users is one of our priorities, but I don't see this change as particular advantage. Again, it's just my opinion, and I am happy to support the change if users and community think it's a good idea.

JanStreffing commented 2 years ago

Presumably those CMOR standards were not made up without weighing the pros and cons of using this or that missval.

What do we gain by not following the community standard? The closer our model output is to CMOR standard, the less problems we are going to have using community tools, and the more our data is usable by the community.

koldunovn commented 2 years ago

Presumably those CMOR standards were not made up without weighing the pros and cons of using this or that missval.

I would not count on it too much :)

What do we gain by not following the community standard? The closer our model output is to CMOR standard, the less problems we are going to have using community tools, and the more our data is usable by the community.

Well, that's a fair point.

JanStreffing commented 2 years ago

I had a quick chat with @dsidoren. He said that the choice of 0 missval was a pragmatic one to get the model up and running early on, when some diagnostics were not fully developed yet, and could e.g. sample accidentally from subsurface mesh points.

He was also in favor of switching now that the diagnostics are much more robust. Whenever you find some time @pgierz ;-)

pgierz commented 2 years ago

Alright. Probably not until tomorrow unfortunately, today has been meeting after meeting after meeting...

pgierz commented 2 years ago

And, dumb question: is there a universal location for the missing value? (io_meandata.F90?) or do I need to go through and write -9e99 in 50 different places?

pgierz commented 2 years ago

Restarting this topic as we recently had it again in a meeting. Maybe we should either sit down together briefly (virtually) or write here pros/cons/stumbling blocks?

Devs: @koldunovn @patrickscholz @dsidoren Users: @JanStreffing @chrisdane