Issue with _FillValue not being written for NetCDF3 files

[hmb1]

Please provide as much of the following information as you can, as applicable to the issue being reported. Naturally, not all information is relevant to every issue, but the more information we have to start, the better!

Environment Information

Linux environment

Feel free to skip this if the issue is related to documentation, a feature request, or general discussion.

• What platform are you using? (please provide specific distribution/version in summary)
  • [X ] Linux
  • Windows
  • OSX
  • Other
  • NA
• 32 and/or 64 bit?
  • 32-bit
  • [X ] 64-bit
• What build system are you using?
  • [ X] autotools (configure)
  • cmake
• Can you provide a sample netCDF file or C code to recreate the issue?
  • Yes (please attach to this issue, thank you!)
  • No
  • Not at this time

Summary of Issue

The following NCO command works in netcdf-4.6.0 and NOT in netcdf-4.6.1
ncap2 -C -v -O -s 'n2=three_dmn_var_dbl;' in.nc foo.nc

I get the following error:

nco_err_exit(): ERROR Short NCO-generated message (usually name of function that triggered error): nco_put_att()
nco_err_exit(): ERROR Error code is -122. Translation into English with nc_strerror(-122) is "NetCDF: Attempt to define fill value when data already exists."
nco_err_exit(): ERROR NCO will now exit with system call exit(EXIT_FAILURE)

When the output is NetCDF4 the command succeeds e.g
ncap2 -4 -C -v -O -s 'n2=three_dmn_var_dbl;' in.nc foo.nc

in.nc is a NetCDF classic file
the variable three_dmn_var_dbl has the following definition:

        double three_dmn_var_dbl(time,lat,lon);
        three_dmn_var_dbl:long_name = "three dimensional record variable of type double";
        three_dmn_var_dbl:units = "watt meter-2";
        three_dmn_var_dbl:_FillValue = -99.;

...Henry

[WardF]

Thanks @hmb1! Can you provide the .nc file you're using, so that I can try to duplicate this issue? Also, when you say netcdf-4.6.1, I presume you mean the master branch from github, correct?

Thanks!

[hmb1]

I picked up the version 4.6.0.1 from your ftp site:
ftp://ftp.unidata.ucar.edu/pub/netcdf/netcdf-4.6.0.tar.gz
..Henry

[hmb1]

Hi Ward,
heres the the cdl file

in.txt

just recreate in.nc with
`ncgen -3 in.txt -o in.nc

...Henry`

[WardF]

Ok, that is version 4.6.0. I will take a look, thanks.

[WardF]

I ask because 4.6.0 is the latest official release, and you indicate that it works. I will test against the latest master branch. Thanks again!

[WardF]

I've confirmed from that link you sent that it is netcdf 4.6.0, which is the latest official release. You can safely stick with 4.6.0, and I will check against our development branch before we put out a new version :).

[WardF]

So I've identified this issue as being a pull request submitted from an unofficial forked branch of netcdf. For now I suggest you revert to the official netcdf (4.6.0), and I will coordinate with the author to get this bug fixed before we accept the pull request. Thanks!

[WardF]

Tagging @czender - Charlie, on the branch where I can duplicate this failure, make check for NCO still passes. Is this bug one of the many caught by make test, or does it represent something that would benefit from an additional test?

[czender]

Yes, to clarify, @hmb1 and I first noticed this and other (possibly related from the same cause) failures on https://github.com/NetCDF-World-Domination-Council/netcdf-c.git branch ejh_rename_is_here (pinging @edhartnett). I asked Henry to report it. NCO's "make check" is pretty useless for you. The real regression test is "make test", which is where this and a few other failures with ejh_rename_is_here will appear. I have not added the "Unidata" tag to the FAILURE string yet because I am being conservative, and am not even sure the problem is in Unidata master (apparently it is not so perhaps we jumped the gun). My hope is that Henry will learn the ropes of reporting issues and cautiously alerting Unidata when something new fails on master.

[WardF]

Thank you for the clarification; this was useful as it alerted us to an issue before it was merged into master, letting us avoid the issue altogether! Thanks very much :)

[edhartnett]

OK, I am trying to reproduce, but it's not happening.

I downloaded the file in.txt and am doing:

ncgen -3 in.txt -o in.nc

This works for branch ejh_rename_is_here.

Also works for ejh_batch.wif.

Do I see the failure from just running this ncgen command? Or is there something else I need to do?

[hmb1]

Hi Ed,
you need to run the command:
ncap2 -C -v -O -s 'n2=three_dmn_var_dbl;' in.nc foo.nc

..Henry

[edhartnett]

OK, I got a clone of the current nco repo to build, with help from @czender. But when I run that ncap2 command using the Unidata master, I get:

 LD_LIBRARY_PATH=/usr/local/nco/lib /usr/local/nco/bin/ncap -C -v -O -s 'n2=three_dmn_var_dbl;' in.nc foo.nc
ERROR NC_ELATEFILL (formerly NC_EFILLVALUE) Attempt to define fill value when data already exists
HINT: NC_ELATEFILL errors can occur when NCO attempts to create, modify, or overwrite a _FillValue attribute for an existing variable in a netCDF4 file. The netCDF4 format (unlike netCDF3) does not permit this. Does your output file need to be netCDF4 or netCDF4_classic format? One workaround is to change the output format to netCDF3 (e.g., ncks -3 in.nc out.nc), edit _FillValue attributes to your heart's content, and then convert back to netCDF4 (e.g., ncks -4 in.nc out.nc). Unfortunately, the netCDF library behavior in this regard changed (near version 4.4.0 with patch NCF-187) and it has proven difficult to workaround the netCDF4 limitaion in all cases with all netCDF4 library versions.
nco_err_exit(): ERROR Short NCO-generated message (usually name of function that triggered error): nco_put_att()
nco_err_exit(): ERROR Error code is -122. Translation into English with nc_strerror(-122) is "NetCDF: Attempt to define fill value when data already exists."
nco_err_exit(): ERROR NCO will now exit with system call exit(EXIT_FAILURE)

So that's the error you're seeing but I expected it to work on the unidata master branch, and produce an error on ejh_rename_is_here. Did I get something wrong?

I will look into the error and see what is causing it...

[czender]

I'm trying to catch-up here. @edhartnett the command should use ncap2 not ncap (which is old and being deprecated).

[edhartnett]

@czender yes I am now using ncap2. Here's the command:

ed@mikado:~/tmp/nco/src/nco++$ ./ncap2 -C -v -O -s 'n2=three_dmn_var_dbl;' in.nc foo.nc
ERROR NC_ELATEFILL (formerly NC_EFILLVALUE) Attempt to define fill value when data already exists
HINT: NC_ELATEFILL errors can occur when NCO attempts to create, modify, or overwrite a _FillValue attribute for an existing variable in a netCDF4 file. The netCDF4 format (unlike netCDF3) does not permit this. Does your output file need to be netCDF4 or netCDF4_classic format? One workaround is to change the output format to netCDF3 (e.g., ncks -3 in.nc out.nc), edit _FillValue attributes to your heart's content, and then convert back to netCDF4 (e.g., ncks -4 in.nc out.nc). Unfortunately, the netCDF library behavior in this regard changed (near version 4.4.0 with patch NCF-187) and it has proven difficult to workaround the netCDF4 limitaion in all cases with all netCDF4 library versions.
nco_err_exit(): ERROR Short NCO-generated message (usually name of function that triggered error): nco_put_att()
nco_err_exit(): ERROR Error code is -122. Translation into English with nc_strerror(-122) is "NetCDF: Attempt to define fill value when data already exists."
nco_err_exit(): ERROR NCO will now exit with system call exit(EXIT_FAILURE)

[hmb1]

yep thats the error

[edhartnett]

And do you get this on the Unidata master branch as well?

[czender]

Yes, I do:

zender@skyglow:~/nco/bm$ ncap2 -C -v -O -s 'n2=three_dmn_var_dbl;' ~/nco/data/in.nc ~/foo.nc
ERROR NC_ELATEFILL (formerly NC_EFILLVALUE) Attempt to define fill value when data already exists
HINT: NC_ELATEFILL errors can occur when NCO attempts to create, modify, or overwrite a _FillValue attribute for an existing variable in a netCDF4 file. The netCDF4 format (unlike netCDF3) does not permit this. Does your output file need to be netCDF4 or netCDF4_classic format? One workaround is to change the output format to netCDF3 (e.g., ncks -3 in.nc out.nc), edit _FillValue attributes to your heart's content, and then convert back to netCDF4 (e.g., ncks -4 in.nc out.nc). Unfortunately, the netCDF library behavior in this regard changed (near version 4.4.0 with patch NCF-187) and it has proven difficult to workaround the netCDF4 limitation in all cases with all netCDF4 library versions.
nco_err_exit(): ERROR Short NCO-generated message (usually name of function that triggered error): nco_put_att()
nco_err_exit(): ERROR Error code is -122. Translation into English with nc_strerror(-122) is "NetCDF: Attempt to define fill value when data already exists."
nco_err_exit(): ERROR NCO will now exit with system call exit(EXIT_FAILURE)

So apparently it is already in master, but IIRC @hmb1 tested 4.6.0 and says that it does not occur there. Henry, please correct me if I'm wrong.

[WardF]

I do not see this error in master on my end. I will double check this morning to confirm.

[hmb1]

Am getting the error with: netCDF 4.6.1-development
and NOT with netcdf-4.5.0
...

[WardF]

Testing now but perhaps I had not pulled down all the latest changes or some other basic mistake. Will follow up momentarily, compiling NCO now.

[czender]

I do not understand why NCO logging is not working. However, on netCDF master, the above command fails with netCDF3 in.nc but succeeds with a netCDF4 version of the same file, i.e.,

zender@skyglow:~$ ncap2 --log_level=5 -C -v -O -s 'n2=three_dmn_var_dbl;' ~/nco/data/in_4.nc ~/foo.nc
zender@skyglow:~$ ncap2 --log_level=5 -C -v -O -s 'n2=three_dmn_var_dbl;' ~/nco/data/in.nc ~/foo.nc
ERROR NC_ELATEFILL (formerly NC_EFILLVALUE) Attempt to define fill value when data already exists
HINT: NC_ELATEFILL errors can occur when NCO attempts to create, modify, or overwrite a _FillValue attribute for an existing variable in a netCDF4 file. The netCDF4 format (unlike netCDF3) does not permit this. Does your output file need to be netCDF4 or netCDF4_classic format? One workaround is to change the output format to netCDF3 (e.g., ncks -3 in.nc out.nc), edit _FillValue attributes to your heart's content, and then convert back to netCDF4 (e.g., ncks -4 in.nc out.nc). Unfortunately, the netCDF library behavior in this regard changed (near version 4.4.0 with patch NCF-187) and it has proven difficult to workaround the netCDF4 limitation in all cases with all netCDF4 library versions.
nco_err_exit(): ERROR Short NCO-generated message (usually name of function that triggered error): nco_put_att()
nco_err_exit(): ERROR Error code is -122. Translation into English with nc_strerror(-122) is "NetCDF: Attempt to define fill value when data already exists."
nco_err_exit(): ERROR NCO will now exit with system call exit(EXIT_FAILURE)

We do want users to have the same _FillValue behavior between netCDF3 and netCDF4, yes?

[WardF]

Interesting, but at least now I can use git bisect to determine where the bug was introduced. I assume something was fumbled on my end checking master, I am of course observing the same issue now.

[czender]

@hmb1 I asked you to check whether the ncap2 test failure occurs in 4.6.0 (not 4.5.0). Did you do so? And what was the result?

[edhartnett]

OK, I can believe that this is happening on master but not in 4.6.0. This is connected no doubt to some of @wkliao's changes, since he did change a bunch of fill stuff for classic format when he added support for nc_set_fill() for classic files.

I made a bunch of changes to fix string fill values, but those are not involved here because this is a classic file.

If we can figure out what is happening here we can put together a C test to make this.

[WardF]

Thanks for the help all, we will get this fixed. @edhartnett I'll follow up over at #844

[WardF]

Breadcrumbs, this is the result of git bisect

---

HEAD is now at be87124 disable CDF-5 feature when size_t is of size 4 bytes
b4a2947 is the first bad commit

---

I haven't looked at this commit yet, but will start looking into it shortly. Tagging in @wkliao as well.

[wkliao]

The ncap2 error message says "HINT: NC_ELATEFILL errors can occur when NCO attempts to create, modify, or overwrite a _FillValue attribute for an existing variable in a netCDF4 file. The netCDF4 format (unlike netCDF3) does not permit this."

Although I have not tested this particular case, I believe my changes make NetCDF3 consistent with NetCDF4, i.e. NetCDF-3 no longer permits setting _FillValue attribute for variables that already exist (or not in the variable's initial define mode).

[wkliao]

Hi @czender Is there a way to make ncap2 to report the code location that spews a NetCDF error?

[czender]

This seems like deja vu. IIRC, we discussed treatment of _FillValue in the 4.5.x development timeframe, and patches to be more stringent with _FillValue were rejected or deferred due to backwards compatibility issues.
@wkliao I don't think so, @hmb1 may know how, but NCO has it's own "error handler" more or less removed from the source of the error. Normally we would run in gdb and read the stack trace to see what calls were made before an exit.

[WardF]

@czender you are correct, I thought we had removed this constraint for the 4.6.0 release due to the fact it was now permissible due to the damage it did to existing workflows. We will need to reverse this if that is in fact the case.

[wkliao]

If I recall correctly, there were 3 issues related to FillValue we discussed before.

1. Whether to allow _FillValue attribute for global variables.
   The latest master branch with support setting/inquiring per variable fill mode for nc3 files #383 allows it.
2. Whether the data type of attribute _FillValue is required to match with the (non-global) variable's.
   support setting/inquiring per variable fill mode for nc3 files #383 enforces it.
3. Enforce NC_ELATEFILL error for NetCDF classic files to be consistent with NetCDF-4 files.
   support setting/inquiring per variable fill mode for nc3 files #383 enforces it.

If NetCDF decided to relax the above limitations 2 and 3, please check lines 839-849 of libsrc/attr.m4 and make changes accordingly.

netcdf-c/libsrc/attr.m4

Line 839 in 712b13e

if (varid != NC_GLOBAL && !strcmp(name, _FillValue)) {

[WardF]

@wkliao thank you for the reminder/refresher; my attention is quite split currently and that is a terrific help! As I recall, the issue is that enforcing previously documented behavior turned into a workflow issue, where a number of tools/workflows existed which violated these behaviors, to the extent we couldn't begin enforcing restrictions without seriously damaging extant workflows.

Thanks very much again, it is all coming back to me.

[wkliao]

I believe it is important to keep the semantics consistent between NetCDF classic and NetCDF4 formats for the APIs they share.

But this is your call.

[WardF]

For now, I think we need to avoid breaking existing workflows; I am going to revert the change for the time being.