netcdf-c icon indicating copy to clipboard operation
netcdf-c copied to clipboard

nc_get_var_string and ncdump throw HDF error on an NC_STRING variable with one unlimited dimension and written with an offset

Open krisfed opened this issue 4 years ago • 18 comments
trafficstars

I am able to create this NC_STRING variable with one limited and one unlimited dimension, and write to it some data with an offset without issue. But when trying to read it back with nc_get_var_string, I am getting a NC_EHDFERR (-101). When trying to use ncdump on the file, I also see "NetCDF: HDF error". However, I am able to read it fine with nc_get_vars_string.

Here are the repro steps (attached here as well: repro.cpp.zip):

#include <iostream>
#include <vector>
#include "netcdf.h"

void checkErrorCode(int status, const char* message){
    if (status != NC_NOERR){
        std::cout << "Error code: " << status << " from " << message << std::endl;
    }
}

int main(int argc, const char * argv[]) {
    

    // set up data
    
    const char** charPointers = new const char*[4];
    std::string* stringData = new std::string[4];
    
    stringData[0] = "a";
    charPointers[0] = stringData[0].c_str();
    
    stringData[1] = "aa";
    charPointers[1] = stringData[1].c_str();
    
    stringData[2] = "b";
    charPointers[2] = stringData[2].c_str();
    
    stringData[3] = "bb";
    charPointers[3] = stringData[3].c_str();
    
    
    int ncid;
    int retval;
    
    // -- WRITE --
    
    retval = nc_create("myfile.nc", NC_NETCDF4, &ncid);
    checkErrorCode(retval, "nc_create");
    
    // Define dimensions
    int dimid_a;
    retval = nc_def_dim(ncid, "a", NC_UNLIMITED, &dimid_a);
    checkErrorCode(retval, "nc_def_dim for 'a'");
    
    int dimid_b;
    retval = nc_def_dim(ncid, "b", 10, &dimid_b);
    checkErrorCode(retval, "nc_def_dim for 'b'");

    int dimids[2] = {dimid_b, dimid_a};
    
    // Define variable
    int varid;
    retval = nc_def_var(ncid, "var", NC_STRING, 2, dimids, &varid);
    checkErrorCode(retval, "nc_def_var");
        
    
    // Put variable
    size_t start[2] = {0, 1};
    size_t count[2] = {2, 2};
    ptrdiff_t stride[2] = {1, 1};
    
    retval = nc_put_vars_string(ncid, varid, start, count, stride, charPointers);
    checkErrorCode(retval, "nc_put_vars_string");
        

    retval = nc_close(ncid);
    checkErrorCode(retval, "nc_close(1)");
    
    
    // -- READ --
    retval = nc_open("myfile.nc", NC_NOWRITE, &ncid);
    checkErrorCode(retval, "nc_open");
    
    // get dimensions
    int ndims, dimids_read[NC_MAX_VAR_DIMS];
    retval = nc_inq_var(ncid, varid, NULL, NULL, &ndims, dimids_read, NULL);
    checkErrorCode(retval, "nc_inq_var");

    // calculate num elements to read
    size_t dimlen = 0;
    size_t num_items = 1;
    for ( int j = 0; j < ndims; ++j ) {
        retval = nc_inq_dimlen(ncid, dimids_read[j], &dimlen);
        checkErrorCode(retval, "nc_inq_dimlen");
        num_items *= dimlen;
    }
    
    // get var
    char** readStrings = new char*[num_items];
    retval = nc_get_var_string(ncid, varid, readStrings);
    checkErrorCode(retval, "nc_get_var_string");

    retval = nc_close(ncid);
    checkErrorCode(retval, "nc_close(2)");
    
    return retval;
}

The output when run:

$ ./a.out 
Error code: -101 from nc_get_var_string
$ ls
a.out*		myfile.nc	repro.cpp
$ ncdump myfile.nc 
netcdf myfile {
dimensions:
	a = UNLIMITED ; // (3 currently)
	b = 10 ;
variables:
	string var(b, a) ;
data:

 var =
NetCDF: HDF error
Location: file <..>/netcdf/ncdump/vardata.c; line 478

As far as I could tell, to reproduce the issue there has to be >1 dimension, one has to be unlimited, and when writing data the offset needs to be in the unlimited dimension. I also only see this with NC_STRING data.

I am seeing this with netcdf 4.7.4 and the above repro steps are run on macOS 10.15.6 (but we are seeing it on other platforms as well).

Is this a known bug or limitation? I know nc_get_var_string has a warning about using it with unlimited dimensions, but this issue seems specific to NC_STRING

krisfed avatar Apr 15 '21 12:04 krisfed

I'm sorry but I suspect this is connected to some long-standing NC_STRING.

Can you instead use fixed length strings? Then turn on nc_def_var_deflate() for the variable, and it will compress away the extra empty characters?

That will work a lot more reliably.

edwardhartnett avatar Apr 15 '21 12:04 edwardhartnett

Thank you for a quick response, Ed! I guess it is one of those things with NC_STRINGs that you warned me to be careful about in the email...

By the fixed length strings do you mean switching to use NC_CHAR or avoiding using NC_STRING with unlimited dimensions? Is it a limitation that needs to be documented?

krisfed avatar Apr 15 '21 19:04 krisfed

Using fixed length NC_CHAR works well and is simple to code. ;-)

edwardhartnett avatar Apr 15 '21 20:04 edwardhartnett

Thanks, Ed - fair enough! But we just want to make sure we are handling this case correctly in case MATLAB users run into it. So any information (e.g. whether it should be treated as a bug or as a limitation) would be much appreciated!

krisfed avatar Apr 16 '21 11:04 krisfed

I did an experiment in which I used char(*) t (i.e. VLEN of t) as the type of the variable. This should be roughly equivalent to using NC_STRING type because internally this is how NC_STRING is stored. Assuming I got it right, then this works to produce the file. However ncdump crashes on it for some reason. I am investigating this latter problem separately. But h5dump on myfile.nc appears correct. I am attaching my program for verification (I converted it to C). It has extension .txt instead of .c so that github will allow the attachment. svlen.txt

DennisHeimbigner avatar Apr 16 '21 21:04 DennisHeimbigner

The attached ncdump/dumplib.c file fixes the ncdump crash. dumplib.txt

DennisHeimbigner avatar Apr 16 '21 21:04 DennisHeimbigner

Thank you for taking a look, Dennis! I can confirm that svlen.c indeed writes and reads the file without any errors using NC_VLEN instead of NC_STRING. So there is some issue with NC_STRING implementation, even though it should be equivalent to vlen of chars?

krisfed avatar Apr 19 '21 16:04 krisfed

That is my guess. So we need to look at the string code and look for ways it is handled differently than vlen.

DennisHeimbigner avatar Apr 19 '21 16:04 DennisHeimbigner

Thank you, @DennisHeimbigner ! Just to clarify your comment about ncdump - do you mean that the attached ncdump/dumplib.c file prevents the above error ("NetCDF: HDF error") from happening when running ncdump on file created with my repro code? OR that there was some other ncdump issue (a crash)?

krisfed avatar Apr 30 '21 20:04 krisfed

It only fixes the immediate crash. It does not fix the problem with NC_STRING that you identified. That problem is still to be solved.

DennisHeimbigner avatar Apr 30 '21 23:04 DennisHeimbigner

Thank you for clarification! I was a bit confused, since I did not observe ncdump crashing with my file from these repro steps :)

I am seeing an ncdump crash on another file though, so I am trying to see if this version of ncdump/dumplib.c makes a difference... looks like it is a bit different from dumplib.c included in netcdf-c 4.7.4 and 4.8.0

krisfed avatar May 03 '21 12:05 krisfed

Hi, Thanks for looking at this earlier! Just wanted to check - there have been no updates, and this is still an active bug, right? We can report that we still observe the issue in 4.8.1...

krisfed avatar Sep 16 '21 16:09 krisfed

Unfortunately yes, that bug still exists. No estimate on when I can look at it.

DennisHeimbigner avatar Sep 16 '21 18:09 DennisHeimbigner

Thank you Dennis for the confirmation! No worries, I realize this is an edge-case-y issue...

krisfed avatar Sep 16 '21 19:09 krisfed

Hi, just checking in on this - still planned for 4.8.2 for now? No worries, I understand it might be lower on the priority.

krisfed avatar Feb 28 '22 11:02 krisfed

Sorry, doing a check-in again - looks like this is still an active issue, but it might get addressed in next release (v4.9.1)?

krisfed avatar Aug 24 '22 21:08 krisfed

@krisfed thanks for checking in; we are working through. It can be tricky to determine what will make it into the release, given the resources we have to work with, but we will try to have this in the 4.9.1 release :)

WardF avatar Aug 25 '22 20:08 WardF

Thank you Ward! Yes I bet there are many things to juggle, thanks for taking a look!

krisfed avatar Aug 26 '22 12:08 krisfed